D10918: taglibextractor: Refactor for better readability

astippich added inline comments. INLINE COMMENTS > michaelh wrote in taglibextractor.cpp:331 > You mean other than lines 225, 233, 240? Yes, but imho we shouldn't even enter the function for mimetypes not having ogg tags. Maybe that is overly careful REPOSITORY R286 KFileMetaData REVISION

D10918: taglibextractor: Refactor for better readability

astippich requested changes to this revision. astippich added a comment. This revision now requires changes to proceed. Looks good imho, just one small comment inline. of course, @mgallien as the maintainer must also give his ok. INLINE COMMENTS > taglibextractor.cpp:331 > +} else { > +

D10918: taglibextractor: Refactor for better readability

astippich added a comment. Btw, I'm not opposed anymore for merging before D10803 , as I need some more time to think about the value types and probably also need to extend the tests. I will adapt to the changes afterwards. REPOSITORY R286

D10292: change 32px icons for playlist shuffle and repeat

astippich added a comment. after D11049 where the frames for the media buttons were removed I think this is quite needed for a consistent look and feel. I also added 32px version for no repeat and no shuffle which I need for a toggle button for #elisa

D10292: change 32px icons for playlist shuffle and repeat

astippich updated this revision to Diff 29110. astippich added a comment. - add more 32px icons REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10292?vs=26512=29110 BRANCH playlist_shuffle_repeat REVISION DETAIL https://phabricator.kde.org/D10292

D10694: epubextractor: Handle multiple subjects better

astippich added a comment. In D10694#221734 , @mgallien wrote: > In D10694#221719 , @michaelh wrote: > > > This is bad! I have learned baloo itself doesn't handle stringlists. Which in my view

D10803: handle more tags in taglibextractor

astippich updated this revision to Diff 28467. astippich added a comment. - remove lyrics and map encodedby to generator REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10803?vs=28300=28467 BRANCH enhance_taglib REVISION DETAIL

D10803: handle more tags in taglibextractor

astippich planned changes to this revision. astippich added a comment. In D10803#213783 , @michaelh wrote: > In D10803#213767 , @astippich wrote: > > > I don't know dolphin works, but given how

D10803: handle more tags in taglibextractor

astippich added a comment. I don't know dolphin works, but given how KFileMetadata is designed, the new tags should be ignored until explicit support is added in dolphin. INLINE COMMENTS > michaelh wrote in properties.h:164 > ? Oops, seems like a wrong copy & paste :) will fix > michaelh

D10803: handle more tags in taglibextractor

astippich added a comment. First of all, please review carefully for ABI compatibility, I do not know what to watch for that. So I decided to walk into the minefield that tags are. Everyone implements it differently. I've implemented some tags that seem to be supported across a wide

D10803: handle more tags in taglibextractor

astippich created this revision. astippich added a reviewer: mgallien. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. astippich requested review of this revision. REVISION SUMMARY adds the ability to read more tags through taglib when

D10293: remove view-media-playlist from preferences icons

This revision was automatically updated to reflect the committed changes. Closed by commit R266:6927b97af007: remove view-media-playlist from preferences icons (authored by astippich). REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10293?vs=26499=27719

D10279: add 24px media-album-cover icon

This revision was automatically updated to reflect the committed changes. Closed by commit R266:ff179872c915: add 24px media-album-cover icon (authored by astippich). REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10279?vs=26495=27718 REVISION DETAIL

D10292: change 32px icons for playlist shuffle and repeat

astippich added a comment. Thanks for the feedback! So the correct fix would be to change the 22px and 24px icons to use circles around the icons? REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D10292 To: astippich, #breeze, #vdg, andreaska, andreask Cc:

D10279: add 24px media-album-cover icon

astippich added a comment. Hi, it would be nice to get some feedback on this. Same for D10293 and D10292 . If this is not the proper way to contribute to breeze icons, please point me in the right direction.

D10365: New icon for Elisa music player

astippich added a comment. I was a little bit hesitant at first, but I'm starting to like the idea of a cassette as an icon for Elisa. But I think the text in the icon is not required. It will usually be so small that no-one can read it. Anyways, Matthieu should have a comment about this.

D10292: change 32px icons for playlist shuffle and repeat

astippich updated this revision to Diff 26512. astippich added a comment. update to 1px of line width, was 1.5 due to the scaling REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10292?vs=26497=26512 BRANCH playlist_shuffle_repeat REVISION DETAIL

D10293: remove view-media-playlist from preferences icons

astippich added a comment. I don't know if it is acceptable to just remove the icon. I also don't know if this is used anywhere. Stumbled upon while looking into icons for the Elisa music player where this icon then creates strange results. I think another workaround would be to create a

D10293: remove view-media-playlist from preferences icons

astippich created this revision. astippich added reviewers: Breeze, VDG, andreaska. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. astippich requested review of this revision. REVISION SUMMARY the view-media-playlist icon in the

D10292: change 32px icons for playlist shuffle and repeat

astippich edited the summary of this revision. REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D10292 To: astippich, #breeze, #vdg, andreaska Cc: #frameworks, michaelh, ngraham

D10292: change 32px icons for playlist shuffle and repeat

astippich added a comment. Before: F5691700: Screenshot_20180204_134019.png F5691702: Screenshot_20180204_134042.png After, which is also how the 24px and lower icons look like: F5691706:

D10292: change 32px icons for playlist shuffle and repeat

astippich created this revision. astippich added reviewers: Breeze, VDG, andreaska. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. astippich requested review of this revision. REVISION SUMMARY the 32px icons for media-playlist-shuffle

D10279: add 24px media-album-cover icon

astippich updated this revision to Diff 26495. astippich added a comment. use a proper feature branch, which I forgot to use before REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10279?vs=26484=26495 BRANCH media-album-cover REVISION DETAIL

D10279: add 24px media-album-cover icon

astippich updated this revision to Diff 26484. astippich added a comment. - do the same thing for dark icons REPOSITORY R266 Breeze Icons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10279?vs=26461=26484 BRANCH master REVISION DETAIL https://phabricator.kde.org/D10279

D10279: add 24px media-album-cover icon

astippich added a comment. It just adds a missing link to an already existing icon, is a screenshot needed? REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D10279 To: astippich, #breeze Cc: ngraham, #frameworks, michaelh

D10279: add 24px media-album-cover icon

astippich created this revision. astippich added a reviewer: Breeze. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. astippich requested review of this revision. REVISION SUMMARY media-album-cover icon is missing for the 24px size,

<    5   6   7   8   9   10