Differential D12932

handle string lists as input
ClosedPublic
Actions

Authored by astippich on May 16 2018, 6:36 PM.

Details

Reviewers
bruns
Commits
R293:03ab297f2512: handle string lists as input
Summary

handles string lists as inputs for baloo by inserting each string in the list separately

Diff Detail

Repository
R293 Baloo
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
astippich created this revision.May 16 2018, 6:36 PM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptMay 16 2018, 6:36 PM
Restricted Application added subscribers: Baloo, kde-frameworks-devel. · View Herald Transcript
astippich requested review of this revision.May 16 2018, 6:36 PM
astippich added a comment.EditedMay 16 2018, 6:37 PM

Unfortunately I couldn't find tests that execute this code path. Do we have test for this somewhere?
This makes D11365 work

bruns added inline comments.May 16 2018, 7:02 PM
src/file/extractor/result.cpp
103

this check is not strictly necessary - if the list is empty, you iterate zero times in the loop. But see below [1]

105

bool shouldBeIndexed = KFMD:PropertyInfo(property).shouldBeIndexed();

avoids the check on each iteration

106

range-based-for loop, please:
for (const auto& val : values) { ... }

107

[1] ... here you miss the check on val.at(i).empty() - the list can contain empty strings.

astippich updated this revision to Diff 34318.May 16 2018, 7:32 PM
  • incorporate feedback
astippich marked 4 inline comments as done.May 16 2018, 7:33 PM

Thanks, and sorry, copied and pasted too quickly without thinking this through

bruns added inline comments.May 16 2018, 7:54 PM
src/file/extractor/result.cpp
90

const auto list = value.toStringList();, to avoid detach.

93

continue;, you don't want to drop the remaining list entries

astippich updated this revision to Diff 34323.May 16 2018, 8:09 PM
  • fix mistakes
bruns added a comment.May 16 2018, 8:19 PM

looks good, although untested

bruns added a comment.May 16 2018, 8:22 PM

Can you do a manual test on an appropriate file, and add the output (before and after) for balooshow -x <file>?

astippich added a comment.May 16 2018, 8:40 PM

with D11365 applied (e.g. multiple entries are string lists) and using a file with multiple artists

balooshow -x testmultiple.opus

      Bitrate: 67000
      Channels: 1
      Duration: 1
      Genre: Genre
      Sample Rate: 48000
      Track Number: 1
      Release Year: 2015
      Comment: Comment
      Artist: artist2, artist1
      Album: Album
      Album Artist: Album Artist
      Composer: Composer
      Lyricist: Lyricist
      Title: Title
      Disc Number: 1

Internal Info
Terms: Maudio Mogg Mopus Mx T2 Taudio X1-67000 X10-album X11-album X11-artist X12-composer X13-lyricist X15-title X2-1 X3-1 X4-genre X5-48000 X6-1 X62-1 X7-2015 X8-comment album artist title 
File Name Terms: Fopus Ftestmultiple opus testmultiple 
XAttr Terms: 
albumArtist: album artist
comment: comment
discNumber: 1
title: title
composer: composer
lyricist: lyricist
channels: 1
duration: 1
bitRate: 67000
trackNumber: 1
releaseYear: 2015
genre: genre
sampleRate: 48000
album: album

after

      Bitrate: 67000
      Channels: 1
      Duration: 1
      Genre: Genre
      Sample Rate: 48000
      Track Number: 1
      Release Year: 2015
      Comment: Comment
      Artist: artist2, artist1
      Album: Album
      Album Artist: Album Artist
      Composer: Composer
      Lyricist: Lyricist
      Title: Title
      Disc Number: 1

Internal Info
Terms: Maudio Mogg Mopus Mx T2 Taudio X1-67000 X10-album X11-album X11-artist X12-composer X13-lyricist X15-title X2-1 X3-1 X4-genre X5-48000 X6-1 X62-1 X7-2015 X8-comment X9-artist1 X9-artist2 album artist artist1 artist2 title 
File Name Terms: Fopus Ftestmultiple opus testmultiple 
XAttr Terms: 
lyricist: lyricist
title: title
discNumber: 1
comment: comment
artist: artist1 artist2
album: album
albumArtist: album artist
genre: genre
sampleRate: 48000
trackNumber: 1
releaseYear: 2015
bitRate: 67000
channels: 1
duration: 1
composer: composer
bruns accepted this revision.May 16 2018, 9:48 PM

Output looks sane for me.

This revision is now accepted and ready to land.May 16 2018, 9:48 PM
Closed by commit R293:03ab297f2512: handle string lists as input (authored by astippich). · Explain WhyMay 17 2018, 7:48 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/file/extractor/result.cpp (13 lines)

Diff 34395

View Options

src/file/extractor/result.cpp

Loading...