D12932: handle string lists as input

This revision was automatically updated to reflect the committed changes. Closed by commit R293:03ab297f2512: handle string lists as input (authored by astippich). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12932?vs=34323&id=34395 REVISION DETAIL https://

D12932: handle string lists as input

bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. Output looks sane for me. REPOSITORY R293 Baloo BRANCH string_lists REVISION DETAIL https://phabricator.kde.org/D12932 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhn

D12932: handle string lists as input

astippich added a comment. 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

D12932: handle string lists as input

bruns added a comment. Can you do a manual test on an appropriate file, and add the output (before and after) for `balooshow -x `? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12932 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, ast

D12932: handle string lists as input

bruns added a comment. looks good, although untested REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12932 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns

D12932: handle string lists as input

astippich updated this revision to Diff 34323. astippich added a comment. - fix mistakes REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12932?vs=34318&id=34323 BRANCH string_lists REVISION DETAIL https://phabricator.kde.org/D12932 AFFECTED FILES src/

D12932: handle string lists as input

bruns added inline comments. INLINE COMMENTS > result.cpp:90 > +bool shouldBeIndexed = > KFileMetaData::PropertyInfo(property).shouldBeIndexed(); > +for (const auto& val : value.toStringList()) > +{ `const auto list = value.toStringList();`, to avoid detach. > result.cp

D12932: handle string lists as input

astippich marked 4 inline comments as done. astippich added a comment. Thanks, and sorry, copied and pasted too quickly without thinking this through REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12932 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhni

D12932: handle string lists as input

astippich updated this revision to Diff 34318. astippich added a comment. - incorporate feedback REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12932?vs=34314&id=34318 BRANCH string_lists REVISION DETAIL https://phabricator.kde.org/D12932 AFFECTED FILE

D12932: handle string lists as input

bruns added inline comments. INLINE COMMENTS > result.cpp:90 > +const QStringList val = value.toStringList(); > +if (val.isEmpty()) > +return; this check is not strictly necessary - if the list is empty, you iterate zero times in the loop. But see below [1] > result

D12932: handle string lists as input

astippich added a comment. Unfortunately I couldn't find tests that execute this code path. Do we have test for this somewhere? REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D12932 To: astippich, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astipp

D12932: handle string lists as input

astippich created this revision. astippich added a reviewer: bruns. Restricted Application added projects: Frameworks, Baloo. Restricted Application added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY handles string lists as inputs for ba