D7700: Show list of tags in PlacesView

nicolasfella added inline comments. INLINE COMMENTS > kossebau wrote in kfileplacesitem.cpp:351 > @nicolasfella Hi. this is passing the tag string both as translation context > as well as the string to translate as well. Is this on purpose? Even more as > the context parameter is ignored

D7700: Show list of tags in PlacesView

kossebau added inline comments. INLINE COMMENTS > kfileplacesitem.cpp:351 > +{ > +KBookmark bookmark = createSystemBookmark(manager, tag, tag, > QUrl(QStringLiteral("tags:/") + tag), QStringLiteral("tag")); > +bookmark.setMetaDataItem(QStringLiteral("tag"), tag); @nicolasfella Hi. this

D7700: Show list of tags in PlacesView

chehrlic added inline comments. INLINE COMMENTS > kfileplacesmodel.cpp:172 > + tags(), > + tagsLister(new KCoreDirLister()) > { While I way playing around with asan, I got a report that tagsLister is leaking. I can't see a deletion of this object anywhere, even in

D7700: Show list of tags in PlacesView

This revision was automatically updated to reflect the committed changes. Closed by commit R241:523acf53dadc: Show list of tags in PlacesView (authored by nicolasfella). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7700?vs=54298=54383 REVISION DETAIL

D7700: Show list of tags in PlacesView

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Yep, works for me too now. REPOSITORY R241 KIO BRANCH arcpatch-D7700 REVISION DETAIL https://phabricator.kde.org/D7700 To: nicolasfella, #dolphin, #kde_applications, alexeymin,

D7700: Show list of tags in PlacesView

nicolasfella updated this revision to Diff 54298. nicolasfella added a comment. Rebase REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7700?vs=48246=54298 BRANCH arcpatch-D7700 REVISION DETAIL https://phabricator.kde.org/D7700 AFFECTED FILES

D7700: Show list of tags in PlacesView

ngraham added a comment. Can you rebase this on current master? I'll give it another shot and report back. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7700 To: nicolasfella, #dolphin, #kde_applications, alexeymin, ngraham, broulik Cc: broulik, kde-frameworks-devel,

D7700: Show list of tags in PlacesView

nicolasfella added a comment. In D7700#382897 , @ngraham wrote: > Bugs found: > > - If there are no tags, creating one does not cause the Tags section to appear in the Places panel until Dolphin is restarted. > - If the Tags section is

D7700: Show list of tags in PlacesView

ngraham added a comment. Bugs found: - If there are no tags, creating one does not cause the Tags section to appear in the Places panel until Dolphin is restarted. - If the Tags section is already visible because there are already some tags, creating a new tag does not cause that new

D7700: Show list of tags in PlacesView

ngraham added a reviewer: broulik. ngraham added a subscriber: broulik. ngraham added a comment. @broulik I believe you were the one who found a bunch of issues last time; can you re-test? I can't remember what your concerns were, sorry! REPOSITORY R241 KIO REVISION DETAIL

D7700: Show list of tags in PlacesView

nicolasfella updated this revision to Diff 48246. nicolasfella added a comment. - Fix REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7700?vs=48245=48246 BRANCH arcpatch-D7700 REVISION DETAIL https://phabricator.kde.org/D7700 AFFECTED FILES

D7700: Show list of tags in PlacesView

nicolasfella updated this revision to Diff 48245. nicolasfella added a comment. - Whitespace-- REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7700?vs=48244=48245 BRANCH arcpatch-D7700 REVISION DETAIL https://phabricator.kde.org/D7700 AFFECTED FILES

D7700: Show list of tags in PlacesView

nicolasfella reopened this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7700 To: nicolasfella, #dolphin, #kde_applications, alexeymin, ngraham Cc: kde-frameworks-devel, bruns, rkflx, mmustac, spoorun, michaelh,

D7700: Show list of tags in PlacesView

nicolasfella updated this revision to Diff 48244. nicolasfella added a comment. Tags List v2.0 Deleted tags are automatically removed from the panel REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7700?vs=45426=48244 BRANCH arcpatch-D7700 REVISION

D7700: Show list of tags in PlacesView

elvisangelaccio added inline comments. INLINE COMMENTS > kfileplacesmodel.h:66 > +UnknownType, > +TagsType > }; This is missing a `///< @since 5.53` comment. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7700 To: nicolasfella, #dolphin,

D7700: Show list of tags in PlacesView

This revision was automatically updated to reflect the committed changes. Closed by commit R241:2ae8a0c374cd: Show list of tags in PlacesView (authored by nicolasfella). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D7700?vs=45414=45426#toc REPOSITORY R241 KIO CHANGES SINCE LAST

D7700: Show list of tags in PlacesView

nicolasfella added inline comments. INLINE COMMENTS > ngraham wrote in kfileplacesmodel.cpp:187 > const. But in fact, does this even need to be a variable? It's only used once. Will fix on push REPOSITORY R241 KIO BRANCH arcpatch-D7700 REVISION DETAIL https://phabricator.kde.org/D7700

D7700: Show list of tags in PlacesView

ngraham added a task: T8349: Improve Places panel usability and presentation. REPOSITORY R241 KIO BRANCH arcpatch-D7700 REVISION DETAIL https://phabricator.kde.org/D7700 To: nicolasfella, #dolphin, #kde_applications, alexeymin, ngraham Cc: kde-frameworks-devel, bruns, rkflx, mmustac,

D7700: Show list of tags in PlacesView

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. A surprisingly small amount of code for a nice feature. Code looks good to me except for one minor thing. This works great in my testing now. I say let's ship it, especially because it

D7700: Show list of tags in PlacesView

nicolasfella updated this revision to Diff 45414. nicolasfella added a comment. - Merge branch 'master' into arcpatch-D7700 REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7700?vs=38100=45414 BRANCH arcpatch-D7700 REVISION DETAIL

D7700: Show list of tags in PlacesView

ngraham added a comment. Me neither. Can you rebase this on current master? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D7700 To: nicolasfella, #dolphin, #kde_applications, alexeymin, ngraham Cc: kde-frameworks-devel, bruns, rkflx, mmustac, spoorun, michaelh,

D7700: Show list of tags in PlacesView

ngraham added a comment. Results of testing: - After I create a new tag, it doesn't show up in the Places panel tag list or under the `tags:/` ioslave until I restart Dolphin. - When I navigate to `tags:/` ioslave using the All Tags item in the Places panel and I click on a tag, its

D7700: Show list of tags in PlacesView

nicolasfella updated this revision to Diff 38100. nicolasfella added a comment. Actually save tag bookmark and check if it already exists before adding REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D7700?vs=37300=38100 BRANCH tags REVISION DETAIL

D7700: Show list of tags in PlacesView

nicolasfella updated this revision to Diff 37300. nicolasfella added a comment. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. Add tags places in KIO REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE