D8434: Created 'remote' section
This revision was automatically updated to reflect the committed changes. Closed by commit R241:3e8eb3746b33: Created remote section (authored by Renato Araujo Oliveira Filho renato.ara...@kdab.com, committed by ngraham). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=22961=22968 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 22961. renatoo added a comment. Updated parent branch REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=22950=22961 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
ngraham added a comment. @renatoo Next! :-) Time to update this one, too. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 22950. renatoo added a comment. Updated parent branch REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=22812=22950 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 22812. renatoo added a comment. Parent branch updated REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=22777=22812 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 22777. renatoo added a comment. Updated from master REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=22765=22777 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 22765. renatoo added a comment. Updated parent branch REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=22454=22765 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo added a dependent revision: D8943: Create 'KFilePlacesModel::convertedUrl' static function. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo added a dependent revision: D8862: Extend API. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo added a dependent revision: D8855: Use Kio::KPlacesModel as source model for PlacesItemModel. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 22454. renatoo added a comment. Updated parent branch REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=22394=22454 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 22394. renatoo added a comment. Updated parent branch REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=22330=22394 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
ngraham added a comment. @dfaure? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 22330. renatoo added a comment. Updated parent branch REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=21934=22330 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
mlaurent accepted this revision. mlaurent added a comment. Seems ok for me now. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo marked 2 inline comments as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 21934. renatoo marked 4 inline comments as done. renatoo added a comment. Fixed typos REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=21809=21934 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kfileplacesmodeltest.cpp:804 > + > +// check if url list is corret after insertion > +QStringList urls; typo: correct > kfileplacesmodeltest.cpp:812 > + > +// check if the new ulr was inserted int the right position (end of > "Remote" section) > +QTRY_COMPARE(spy_inserted.count(), 1); typo: URL typo: *in* the > kfileplacesmodeltest.cpp:819 > + > +// check if the new ulr has the right group "Remote" > +const QModelIndex index = m_places->index(expectedRow, 0); typo: URL > kfileplacesitem_p.h:48 > PlacesType = 0, > -RecentlySavedType = 1, > -SearchForType = 2, > -DevicesType = 3, > -RemovableDevicesType = 4 > +RemoteType = 1, > +RecentlySavedType = 2, If the actual numbers don't matter (as shown by this commit), why do we even number these explicitly? I would just remove all the =0, =1, =2... values. It will make the diff smaller in the next such commit ;) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent, dfaure Cc: dfaure, elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 21809. renatoo added a comment. Updated parent branch REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=21760=21809 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
mlaurent accepted this revision. mlaurent added a comment. Seems good for me REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff, mlaurent Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 21760. renatoo added a comment. Updated parent branch REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=21757=21760 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 21757. renatoo added a comment. Fixed code style REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=21697=21757 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp autotests/kfileplacesviewtest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin, mwolff Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
ngraham accepted this revision. ngraham added a comment. Lovely. Looks good to me, too! REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo edited the summary of this revision. renatoo added dependencies: D8332: Added baloo urls into places model, D8348: Add a section for removable devices. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
ngraham added a comment. Can you add "Depends on D" for each other patch that this requires? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
mwolff accepted this revision. mwolff added a comment. This revision is now accepted and ready to land. lgtm, but please wait a bit, maybe someone else wants to chime in? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin, mwolff Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 21697. renatoo added a comment. Added more test cases Renamed enum to match group name REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=21694=21697 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
mwolff added inline comments.
INLINE COMMENTS
> kfileplacesmodeltest.cpp:784
> +// insert a new network url
> +m_places->addPlace(QStringLiteral("My Shared"), QUrl(
> QStringLiteral("ftp://192.168.1.1/ftp;)), QString(), QString(),
> QModelIndex());
> +
please add URLs for the following schemas, too:
smb
sftp
fish
webdav
> kfileplacesitem_p.h:48
> PlacesType = 0,
> -RecentlySavedType = 1,
> -SearchForType = 2,
> -DevicesType = 3,
> -RemovableDevicesType = 4
> +NetworkType = 1,
> +RecentlySavedType = 2,
this should probably also be called `RemoteType` now, no?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8434
To: renatoo, ngraham, #frameworks, #dolphin
Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo updated this revision to Diff 21694. renatoo added a comment. Created unit test for remote ulrs REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8434?vs=21693=21694 REVISION DETAIL https://phabricator.kde.org/D8434 AFFECTED FILES autotests/kfileplacesmodeltest.cpp src/filewidgets/kfileplacesitem.cpp src/filewidgets/kfileplacesitem_p.h To: renatoo, ngraham, #frameworks, #dolphin Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
ngraham added a comment. +1 on Remote. We really need to remove the duplicated Places test that's the header for the whole widget, though. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo added a comment. In https://phabricator.kde.org/D8434#162786, @mwolff wrote: > Well, but if we use `Network` for `remote://` already, then the group should also have this label, no? I don't see an issue with this, really. On the contrary - maybe we could in the future remove the `remote://` link and let the category header react to a click, such that it will show you e.g. all remote devices. That would remove the duplication then. I think duplicate In https://phabricator.kde.org/D8434#162002, @ngraham wrote: > +1 for the idea! Needs more screenshots. :) screenshot added REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
D8434: Created 'remote' section
renatoo retitled this revision from "Created 'shared' section" to "Created 'remote' section". renatoo edited the summary of this revision. renatoo edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D8434 To: renatoo, ngraham, #frameworks, #dolphin Cc: elvisangelaccio, mwolff, mlaurent, #frameworks
