D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham added a comment.


  It was done in the interests of getting something in rather than nothing, 
since my initial larger list was controversial. I would favor adding Documents 
too, FWIW. Seems like others felt similarly. Wanna submit a patch? :)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: kde-frameworks-devel, acrouthamel, abetts, huoni, markg, fabiank, 
progwolff, broulik, michaelh, ngraham, bruns

D11768: Add Desktop and Downloads to the default list of Places

[Andrew Crouthamel]

acrouthamel added a comment.
Herald added a subscriber: kde-frameworks-devel.


  Hey Nate, is there a reason why Documents was not added with this patch? I 
always end up adding it. I was reading through D10245#201638 
 and the discussion seemed in favor 
of adding it, along with the Desktop and Downloads.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: kde-frameworks-devel, acrouthamel, abetts, huoni, markg, fabiank, 
progwolff, broulik, michaelh, ngraham, bruns

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham closed this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: abetts, huoni, markg, fabiank, progwolff, broulik, michaelh, bruns

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham removed a dependency: D11767: Remove Recently Saved This Month and 
Recently Saved Last Month entries by default.

REPOSITORY
  R241 KIO

BRANCH
  add-desktop-and-downloads (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: abetts, huoni, markg, fabiank, progwolff, broulik, michaelh, bruns

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

BRANCH
  add-desktop-and-downloads (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: abetts, huoni, markg, fabiank, progwolff, broulik, michaelh, bruns

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham added a task: T8349: Improve Places panel usability and presentation.

REPOSITORY
  R241 KIO

BRANCH
  add-desktop-and-downloads (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: abetts, huoni, markg, fabiank, progwolff, broulik, michaelh, bruns

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham added a comment.


  Heh, thanks. Renato said he'd fix the tests soon, FWIW.

REPOSITORY
  R241 KIO

BRANCH
  add-desktop-and-downloads (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: abetts, huoni, markg, fabiank, progwolff, broulik, michaelh, bruns

D11768: Add Desktop and Downloads to the default list of Places

[Mark Gaiser]

markg accepted this revision.
markg added a comment.
This revision is now accepted and ready to land.


  Blocking it any longer seems rude to me :)

REPOSITORY
  R241 KIO

BRANCH
  add-desktop-and-downloads (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: abetts, huoni, markg, fabiank, progwolff, broulik, michaelh, bruns

D11768: Add Desktop and Downloads to the default list of Places

[Andres Betts]

abetts added a comment.


  I would say remove search for images audio files and video and move them to 
the places location

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: abetts, huoni, markg, fabiank, progwolff, broulik, michaelh, bruns

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: huoni, markg, fabiank, progwolff, broulik, michaelh, ngraham, bruns

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham set the repository for this revision to R241 KIO.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: huoni, markg, fabiank, progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham edited the summary of this revision.

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: huoni, markg, fabiank, progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Julian Wolff]

progwolff accepted this revision.

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg, progwolff
Cc: huoni, markg, fabiank, progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham marked 2 inline comments as done.

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg
Cc: huoni, markg, fabiank, progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Huon Imberger]

huoni added a comment.


  FWIW, I run Arch, and never use Desktop View and therefore the Desktop folder.
  I //do// have `~/Desktop`, but I do not have the places entries by default.
  
  I definitely +1 this change, but it has to be robust and deal with any 
combination of pre-existing distro defaults.

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg
Cc: huoni, markg, fabiank, progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham updated this revision to Diff 30891.
ngraham added a comment.


  Only create user place bookmarks if their target directories already exist

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11768?vs=30799&id=30891

BRANCH
  add-desktop-and-downloads (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11768

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.cpp

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg
Cc: markg, fabiank, progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham added a comment.


  I sent an email to Renato asking if he can resolve the test failures.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg
Cc: markg, fabiank, progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Mark Gaiser]

markg requested changes to this revision.
markg added a comment.
This revision now requires changes to proceed.


  I don't know why, but i already have this by default in my Dolphin... 
(ArchLinux user here, i don't think they add it explicitly as they try to stay 
as true to upstream as possible).
  So, err, don't know why.
  
  Anyhow, having them by default is a big +1 from me.
  
  Yet i still give a -1.. The reason for that is simple. Ever since the 
placesmodel changes in KIO and Dolphin, a unittest for that fails. Fix that 
first.
  I did have a look at it, but quite some placesmodel magic changed so it's 
better if one of the authors of those changes takes a look.
  For reference:, look at all the recent builds of Dolphin and KIO.
  Dolphin: 
https://build.kde.org/job/Applications%20dolphin%20kf5-qt5%20SUSEQt5.9/
  KIO: https://build.kde.org/job/Frameworks%20kio%20kf5-qt5%20SUSEQt5.10/

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg, markg
Cc: markg, fabiank, progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham added a comment.


  In D11768#236365 , @fabiank wrote:
  
  > In D11768#236349 , @ngraham 
wrote:
  >
  > > In Neon and Kubuntu, ~/Desktop exists for a new user by default. Is this 
not the case in other distros?
  >
  >
  > I'm not sure what is currently done, but I'm pretty sure in Chakra we used 
to create localized versions (and I think even Ubuntu did that), so instead of 
~/Desktop,  a German user would have ~/Schreibtisch. There's actually an xdg 
standard for this, https://www.freedesktop.org/wiki/Software/xdg-user-dirs. 
~~Afaik, Qt supports this with QStandardPaths, e.g. 
QStandardPaths::DesktopLocation.~~ Actually, scratch that, it doesn't support 
xdg-user-dirs, and returns a generic value.
  
  
  This patch uses `QStandardPaths::DesktopLocation()`, so that case is handled. 
In discussions here, I was just referring to "~/Desktop" as a placeholder for 
the appropriate location.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg
Cc: fabiank, progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Fabian Kosmale]

fabiank added a comment.


  In D11768#236349 , @ngraham wrote:
  
  > In Neon and Kubuntu, ~/Desktop exists for a new user by default. Is this 
not the case in other distros?
  
  
  I'm not sure what is currently done, but I'm pretty sure in Chakra we used to 
create localized versions (and I think even Ubuntu did that), so instead of 
~/Desktop,  a German user would have ~/Schreibtisch. There's actually an xdg 
standard for this, https://www.freedesktop.org/wiki/Software/xdg-user-dirs. 
Afaik, Qt supports this with QStandardPaths, e.g. 
QStandardPaths::DesktopLocation.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg
Cc: fabiank, progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Julian Wolff]

progwolff added a comment.


  In D11768#236348 , @broulik wrote:
  
  > > Still, that folder exists by default
  >
  > Are you sure? It might be created by some weird startup script or something 
else. Also, a user can choose to change the location of the Desktop folder. 
Just wanting to make sure this change (which I'm totally in favor of, don't get 
me wrong) doesn't break or have unwanted side-effects.
  
  
  It might exist by default on some distributions, but we cannot assume that it 
is created on all systems. Arch Linux, just to give one example, will not 
create any folders for you. I imagine that it can be quite annoying to see a 
warning on the first start of dolphin, just because a folder is missing that 
the user didn't event want to have.
  
  I like the change in general, but please add a check if the folder exists 
before creating the bookmark.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg
Cc: progwolff, broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham added a comment.


  In Neon and Kubuntu, ~/Desktop exists for a new user by default. Is this not 
the case in other distros?
  
  I think a user who goes and figures out how to make Folder View point to a 
different folder than ~/Desktop is the kind of user who can figure out how to 
edit and delete Places panel entries.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg
Cc: broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Kai Uwe Broulik]

broulik added a comment.


  > Still, that folder exists by default
  
  Are you sure? It might be created by some weird startup script or something 
else. Also, a user can choose to change the location of the Desktop folder. 
Just wanting to make sure this change (which I'm totally in favor of, don't get 
me wrong) doesn't break or have unwanted side-effects.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg
Cc: broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> broulik wrote in kfileplacesmodel.cpp:245
> Desktop KIO claims that this may return $HOME if the Desktop folder does not 
> exist, can you confirm/invalidate that?

If I delete ~/Desktop, trying to access it via the Places entry gives me a red 
KMessageWidget that says "The file or folder /home/test/Desktop does not exist."

Still, that folder exists by default, and I think the kind of user who deletes 
their Desktop folder is the kind of user we're not targeting with this change.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg
Cc: broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Kai Uwe Broulik]

broulik added a comment.


  +1

INLINE COMMENTS

> kfileplacesmodel.cpp:245
> +  QStringLiteral("Desktop"), 
> I18N_NOOP2("KFile System Bookmarks", "Desktop"),
> +  
> QUrl::fromLocalFile(QStandardPaths::writableLocation(QStandardPaths::DesktopLocation)),
>  QStringLiteral("user-desktop"));
> +

Desktop KIO claims that this may return $HOME if the Desktop folder does not 
exist, can you confirm/invalidate that?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg
Cc: broulik, michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham edited the test plan for this revision.
ngraham set the repository for this revision to R241 KIO.
Restricted Application added a project: Frameworks.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg
Cc: michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham edited the summary of this revision.
ngraham added a dependency: D11767: Remove Recently Saved This Month and 
Recently Saved Last Month entries by default.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D11768

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg
Cc: michaelh, ngraham

D11768: Add Desktop and Downloads to the default list of Places

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Dolphin, Gwenview, Frameworks, VDG.
ngraham requested review of this revision.

REVISION SUMMARY
  Add Desktop and Downloads to the default list of places, for the following 
reasons:
  
  1. The Desktop and Downloads folders are heavily used by average users, and 
making them accessible by default from Dolphin and file open/save dialogs is a 
big usability win
  2. We'll have space to do it without cluttering up the panel from having 
removed some un-useful entries in D11767: Remove Recently Saved This Month and 
Recently Saved Last Month entries by default 


TEST PLAN
  Created a new user account, logged into it, and opened Dolphin:
  [image goes here]

BRANCH
  add-desktop-and-downloads (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11768

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.cpp

To: ngraham, #dolphin, #gwenview, #frameworks, #vdg