D15739: [Places panel] Don't show Root by default

[Stefan Brüns]

bruns added a comment.


  In D15739#486047 , @fvogt wrote:
  
  > If adding such a special case makes sense, yes. I'd even argue about 
'mntent.mnt_dir == "/" && !isKnownFilesystem(mntent)' or something like that to 
ensure that an entry for `/` is always provided.
  
  
  Guaranteeing there will be an "/" entry is hard, as this can be provided by 
different backends. And I am not sure how e.g. dolphin would deal with multiple 
devices providing "/".

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: andriusr, fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, 
ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, broulik, 
acrouthamel, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Fabian Vogt]

fvogt added a comment.


  In D15739#486043 , @bruns wrote:
  
  > In D15739#486020 , @fvogt wrote:
  >
  > > In D15739#486009 , @bruns 
wrote:
  > >
  > > > It actually already does, via the fstab backend (currently network 
(SMB/NFS) and various fuse mounts). Fstab in this case also includes everything 
in MTAB, should be quite easy to extend this.
  > > >
  > > > What is the output of `cat /proc/self/mounts` (fell free to remove 
anything irrelevant, like cgroups)?
  > >
  > >
  > > The entries involved in / are these:
  > >
  > >   /dev/sr0 /run/initramfs/live iso9660 ro,relatime 0 0
  > >   /dev/loop0 /run/initramfs/squashfs_container squashfs ro,relatime 0 0
  > >   /dev/loop1 /run/rootfsbase ext4 ro,relatime,data=ordered 0 0
  > >   LiveOS_rootfs / overlay 
rw,relatime,lowerdir=/run/rootfsbase,upperdir=/run/overlayfs/rw,workdir=/run/overlayfs/work
 0 0
  > >
  >
  >
  > Hm, loop1 is the `lowerdir` of the overlay - how are loop0 and sr0 
involved, are these the backing files?
  
  
  /dev/sr0 contains a squashfs, which is visible as /dev/loop0.
  /dev/loop0 contains an ext4 image, which is visible as /dev/loop1.
  
  So it's doubly indirect.
  
  > Though, the relevant part is `mntent.mnt.type == "overlay"` and 
`mntent.mnt_dir == "/"`. Matching for "overlay" is probably sufficient.
  
  If adding such a special case makes sense, yes. I'd even argue about 
'mntent.mnt_dir == "/" && isKnownFilesystem(mntent)' or something like that to 
ensure that an entry for `/` is always provided.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: andriusr, fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, 
ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, broulik, 
acrouthamel, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Stefan Brüns]

bruns added a comment.


  In D15739#486020 , @fvogt wrote:
  
  > In D15739#486009 , @bruns wrote:
  >
  > > It actually already does, via the fstab backend (currently network 
(SMB/NFS) and various fuse mounts). Fstab in this case also includes everything 
in MTAB, should be quite easy to extend this.
  > >
  > > What is the output of `cat /proc/self/mounts` (fell free to remove 
anything irrelevant, like cgroups)?
  >
  >
  > The entries involved in / are these:
  >
  >   /dev/sr0 /run/initramfs/live iso9660 ro,relatime 0 0
  >   /dev/loop0 /run/initramfs/squashfs_container squashfs ro,relatime 0 0
  >   /dev/loop1 /run/rootfsbase ext4 ro,relatime,data=ordered 0 0
  >   LiveOS_rootfs / overlay 
rw,relatime,lowerdir=/run/rootfsbase,upperdir=/run/overlayfs/rw,workdir=/run/overlayfs/work
 0 0
  >
  
  
  Hm, loop1 is the `lowerdir` of the overlay - how are loop0 and sr0 involved, 
are these the backing files?
  
  Though, the relevant part is `mntent.mnt.type == "overlay"` and 
`mntent.mnt_dir == "/"`. Matching for "overlay" is probably sufficient.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: andriusr, fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, 
ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, broulik, 
acrouthamel, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Fabian Vogt]

fvogt added a comment.


  In D15739#486009 , @bruns wrote:
  
  > In D15739#485983 , @fvogt wrote:
  >
  > > Even if all (block) devices and their mountpoints were shown in the 
devices view, there would be no equivalent of "/". One loop device provides the 
read-only base for /, but that's actually more confusing than the current state 
as it looks like /, but is only a read-only view of "the past".
  > >
  > > I guess solid needs to gain support for mountpoints not backed by devices?
  >
  >
  > It actually already does, via the fstab backend (currently network 
(SMB/NFS) and various fuse mounts). Fstab in this case also includes everything 
in MTAB, should be quite easy to extend this.
  >
  > What is the output of `cat /proc/self/mounts` (fell free to remove anything 
irrelevant, like cgroups)?
  
  
  The entries involved in / are these:
  
/dev/sr0 /run/initramfs/live iso9660 ro,relatime 0 0
/dev/loop0 /run/initramfs/squashfs_container squashfs ro,relatime 0 0
/dev/loop1 /run/rootfsbase ext4 ro,relatime,data=ordered 0 0
LiveOS_rootfs / overlay 
rw,relatime,lowerdir=/run/rootfsbase,upperdir=/run/overlayfs/rw,workdir=/run/overlayfs/work
 0 0

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: andriusr, fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, 
ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, broulik, 
acrouthamel, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Stefan Brüns]

bruns added a comment.


  In D15739#485983 , @fvogt wrote:
  
  > Even if all (block) devices and their mountpoints were shown in the devices 
view, there would be no equivalent of "/". One loop device provides the 
read-only base for /, but that's actually more confusing than the current state 
as it looks like /, but is only a read-only view of "the past".
  >
  > I guess solid needs to gain support for mountpoints not backed by devices?
  
  
  It actually already does, via the fstab backend (currently network (SMB/NFS) 
and various fuse mounts). Fstab in this case also includes everything in MTAB, 
should be quite easy to extend this.
  
  What is the output of `cat /proc/self/mounts` (fell free to remove anything 
irrelevant, like cgroups)?

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: andriusr, fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, 
ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, broulik, 
acrouthamel, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Andrius da Costa Ribas]

andriusr added a comment.


  In D15739#485978 , @fvogt wrote:
  
  > In D15739#485976 , @ngraham 
wrote:
  >
  > > In D15739#485975 , @fvogt 
wrote:
  > >
  > > > No, there's in fact no devices section at all as it would be empty. The 
live cd itself is marked as ignored as there's nothing useful on there and it 
can't be ejected and the other layers are mounted from loop devices which 
udisks apparently ignores as well.
  > >
  > >
  > > That seems like a bug in the way the devices list is populated or 
displayed. There can't be no devices, that's silly.
  >
  >
  > Well, the only mount of interest is the overlay mount of /, which is not 
backed by any (block) device but rather by two filesystems. AFAICT udisks 
doesn't care about this kind of mount, not sure what solid does.
  >
  > Showing an overlay mount as "Device" is also somewhat wrong IMO.
  
  
  On Windows, we show "subst" drives as a Device, which is a bit similar to 
that, if solid could list those "virtual" devices so they're available on the 
places panel it would be useful for easy access to those mountpoints imho.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: andriusr, fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, 
ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, broulik, 
acrouthamel, kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  Could be. I'm not at all an expert on Solid; I just think that it never makes 
sense to have a Places Panel with no Devices section. How to fix that is 
definitely best left up to people far smarter than me. :)

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, 
romangg, bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Fabian Vogt]

fvogt added a comment.


  In D15739#485979 , @ngraham wrote:
  
  > In D15739#485978 , @fvogt wrote:
  >
  > > Well, the only mount of interest is the overlay mount of /, which is not 
backed by any (block) device but rather by two filesystems. AFAICT udisks 
doesn't care about this kind of mount, not sure what solid does.
  > >
  > > Showing an overlay mount as "Device" is also somewhat wrong IMO.
  >
  >
  > I'm saying that regardless of the technical details of the backend, it 
never makes sense to not show any devices. From the user's perspective, there 
is always a device of some sort, regardless of its underlying configuration. 
There can't not be a device. That doesn't make sense; software requires 
hardware.
  
  
  Even if all (block) devices and their mountpoints were shown in the devices 
view, there would be no equivalent of "/". One loop device provides the 
read-only base for /, but that's actually more confusing than the current state 
as it looks like /, but is only a read-only view of "the past".
  
  I guess solid needs to gain support for mountpoints not backed by devices?

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, 
romangg, bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  In D15739#485978 , @fvogt wrote:
  
  > Well, the only mount of interest is the overlay mount of /, which is not 
backed by any (block) device but rather by two filesystems. AFAICT udisks 
doesn't care about this kind of mount, not sure what solid does.
  >
  > Showing an overlay mount as "Device" is also somewhat wrong IMO.
  
  
  I'm saying that regardless of the technical details of the backend, it never 
makes sense to not show any devices. From the user's perspective, there is 
always a device of some sort, regardless of its underlying configuration. There 
can't not be a device. That doesn't make sense; software requires hardware.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, 
romangg, bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Fabian Vogt]

fvogt added a comment.


  In D15739#485976 , @ngraham wrote:
  
  > In D15739#485975 , @fvogt wrote:
  >
  > > No, there's in fact no devices section at all as it would be empty. The 
live cd itself is marked as ignored as there's nothing useful on there and it 
can't be ejected and the other layers are mounted from loop devices which 
udisks apparently ignores as well.
  >
  >
  > That seems like a bug in the way the devices list is populated or 
displayed. There can't be no devices, that's silly.
  
  
  Well, the only mount of interest is the overlay mount of /, which is not 
backed by any (block) device but rather by two filesystems. AFAICT udisks 
doesn't care about this kind of mount, not sure what solid does.
  
  Showing an overlay mount as "Device" is also somewhat wrong IMO.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, 
romangg, bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  In D15739#485975 , @fvogt wrote:
  
  > No, there's in fact no devices section at all as it would be empty. The 
live cd itself is marked as ignored as there's nothing useful on there and it 
can't be ejected and the other layers are mounted from loop devices which 
udisks apparently ignores as well.
  
  
  That seems like a bug in the way the devices list is populated or displayed. 
There can't be no devices, that's silly.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, 
romangg, bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Fabian Vogt]

fvogt added a comment.


  In D15739#485960 , @ngraham wrote:
  
  > In D15739#485949 , @fvogt wrote:
  >
  > > I just noticed this in openQA runs from a live medium. There is no 
single-click way to get to the root directory anymore, as / is mounted as an 
overlayfs of a tmpfs and a squashfs container.
  > >
  > > Not sure what the best way to improve this is, any idea?
  >
  >
  > So there's no entry in the Devices section that goes to `/`?
  
  
  No, there's in fact no devices section at all as it would be empty. The live 
cd itself is marked as ignored as there's nothing useful on there and it can't 
be ejected and the other layers are mounted from loop devices which udisks 
apparently ignores as well.

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, 
romangg, bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  In D15739#485949 , @fvogt wrote:
  
  > I just noticed this in openQA runs from a live medium. There is no 
single-click way to get to the root directory anymore, as / is mounted as an 
overlayfs of a tmpfs and a squashfs container.
  >
  > Not sure what the best way to improve this is, any idea?
  
  
  So there's no entry in the Devices section that goes to `/`?

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, 
romangg, bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Fabian Vogt]

fvogt added a comment.


  I just noticed this in openQA runs from a live medium. There is no 
single-click way to get to the root directory anymore, as / is mounted as an 
overlayfs of a tmpfs and a squashfs container.
  
  Not sure what the best way to improve this is, any idea?

REPOSITORY
  R241 KIO

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: fvogt, meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, 
romangg, bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Méven Car]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:639ba4ca8e83: [Places panel] Dont show Root by 
default (authored by meven).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15739?vs=59751=60490

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

AFFECTED FILES
  autotests/kfileplacesmodeltest.cpp
  autotests/kfileplacesviewtest.cpp
  src/filewidgets/kfileplacesmodel.cpp

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Elvis Angelaccio]

elvisangelaccio accepted this revision.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D15739

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham, elvisangelaccio
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  Lovely. :)

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D15739

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Méven Car]

meven added a dependent revision: D21789: Fix tests relating to the removal of 
the Root Place in D15739.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D15739

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Méven Car]

meven updated this revision to Diff 59751.
meven added a comment.


  - Adapt test to remove Root in the Places by default

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15739?vs=59750=59751

BRANCH
  arcpatch-D15739

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

AFFECTED FILES
  autotests/kfileplacesmodeltest.cpp
  autotests/kfileplacesviewtest.cpp
  src/filewidgets/kfileplacesmodel.cpp

To: meven, #dolphin, #vdg, tcanabrava, ngraham
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  <3
  
  Nice huge diff! You see why I had trouble with it, maybe. :) I am very 
impressed that you pulled this off so quickly though!

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D15739

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Méven Car]

meven updated this revision to Diff 59750.
meven added a comment.


  - Adapt test to remove Root in the Places by default

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15739?vs=59749=59750

BRANCH
  arcpatch-D15739

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

AFFECTED FILES
  autotests/kfileplacesmodeltest.cpp
  autotests/kfileplacesviewtest.cpp
  src/filewidgets/kfileplacesmodel.cpp

To: meven, #dolphin, #vdg, tcanabrava, ngraham
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Méven Car]

meven commandeered this revision.
meven added a reviewer: ngraham.
meven added a comment.


  Taking over this.
  Dolphin test fix patch is coming too.

REPOSITORY
  R241 KIO

BRANCH
  arcpatch-D15739

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

To: meven, #dolphin, #vdg, tcanabrava, ngraham
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Méven Car]

meven updated this revision to Diff 59749.
meven added a comment.
This revision is now accepted and ready to land.


  - Adapt test to remove Root in the Places by default

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15739?vs=42276=59749

BRANCH
  arcpatch-D15739

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

AFFECTED FILES
  autotests/kfileplacesmodeltest.cpp
  src/filewidgets/kfileplacesmodel.cpp

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  If you'd like to, I'm more than happy to hand it off. Removing Root is 
trivial, but adjusting the autotest has been a big pain. It explicitly looks 
for Root and I've had a tough time changing it appropriately. I've sunk a 
number of hours into it without success, so if you'd like to take over, please 
feel free.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Méven Car]

meven added a comment.


  gentle ping @ngraham 
  How ironic ;)
  
  Unless you would rather let someone else take care of this.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: meven, elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, 
bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, LeGast00n, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  In D15739#356449 , 
@elvisangelaccio wrote:
  
  > In D15739#356003 , @ngraham 
wrote:
  >
  > > This blows up the places tests; will fix them in this patch.
  >
  >
  > It will also break the test in dolphin :(
  
  
  I know, and I'll probably have to fix those with ifdefs :(

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, bruns, 
davidedmundson, abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, 
michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Elvis Angelaccio]

elvisangelaccio added a comment.


  In D15739#356003 , @ngraham wrote:
  
  > This blows up the places tests; will fix them in this patch.
  
  
  It will also break the test in dolphin :(

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: elvisangelaccio, Codezela, davidc, tcanabrava, ndavis, romangg, bruns, 
davidedmundson, abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, 
michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  The image in this patch is out of date since it was added before other 
changes were made; I need to update it and will do so later today.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: Codezela, davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, 
abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Andrew Crouthamel]

acrouthamel added a comment.


  In D15739#331468 , @broulik wrote:
  
  > +1 it's an entry I always hide when I setup someone's computer
  >
  > > you can still get to / on your machine in one click using the appropriate 
disk entry on the bottom of the Places panel.
  >
  > Which I think shouldn't be there, but I'm too afraid of touching Solid's 
ignore rules ever again.
  
  
  You advocate for
  
  In D15739#356313 , @Codezela wrote:
  
  > In D15739#356310 , @ndavis wrote:
  >
  > > In D15739#356309 , @Codezela 
wrote:
  > >
  > > > i know that the icons size is adjustable i mean make the default more 
bigger first time u run dolphin this icons is so small "out of the box 
experience"
  > > >  the other thing  i mean we may keep the root partion icon on the 
devices sections and make it fixed position above all other mounted drives so 
even any hd mounted the root icons still top
  > > >  i hope i explain well
  > > >  sorry i know my English is not so good
  > >
  > >
  > > It's ok, I understand now. Those seem like good ideas.
  >
  >
  > sorry but i added some picture after
  >
  > do u see this is my places section
  >  F6398445: Screenshot_20181108_203553.png 

  >  can u tell me where is my root
  >  and the phone icon is on top
  
  
  It will have a new icon as Nate linked to earlier.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: Codezela, davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, 
abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Hazem Salem]

Codezela added a comment.


  In D15739#356310 , @ndavis wrote:
  
  > In D15739#356309 , @Codezela 
wrote:
  >
  > > i know that the icons size is adjustable i mean make the default more 
bigger first time u run dolphin this icons is so small "out of the box 
experience"
  > >  the other thing  i mean we may keep the root partion icon on the devices 
sections and make it fixed position above all other mounted drives so even any 
hd mounted the root icons still top
  > >  i hope i explain well
  > >  sorry i know my English is not so good
  >
  >
  > It's ok, I understand now. Those seem like good ideas.
  
  
  sorry but i added some picture after
  
  do u see this is my places section
  F6398445: Screenshot_20181108_203553.png 

  can u tell me where is my root
  and the phone icon is on top

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: Codezela, davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, 
abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Noah Davis]

ndavis added a comment.


  In D15739#356309 , @Codezela wrote:
  
  > In D15739#356269 , @ndavis wrote:
  >
  > > In D15739#356026 , @Codezela 
wrote:
  > >
  > > > maybe there is 2 alternatives
  > > >
  > > > 1. remove the icon from devices section and keep it in places section 
with some icon change in the top one
  > >
  > >
  > > You mean hide the root partition from Devices and keep the Root Places 
bookmark? That's what I currently do. I hide my `/` and `/home` partitions and 
use Places to get to those directories.
  > >
  > > > 2. keep it in the places section but change the label to root or system 
and it will always be top of all other hd with the lock emblem in it and remove 
the places one and we need to make the icons little bigger in this  side bar 
what do you think or i come oo late
  > >
  > > This confuses me. You say to keep it in Places, but remove it from 
Places? Keep what above other hard drives?
  > >
  > > BTW, the sidebar icons do have adjustable sizes. Perhaps it should be 
easier to find, but if you right click in an area without something selectable 
(e.g., a section label), you can go to "Icon Size" and change the size to 16, 
22, 32 or 48 px.
  >
  >
  > i know that the icons size is adjustable i mean make the default more 
bigger first time u run dolphin this icons is so small "out of the box 
experience"
  >  the other thing  i mean we may keep the root partion icon on the devices 
sections and make it fixed position above all other mounted drives so even any 
hd mounted the root icons still top
  >  i hope i explain well
  >  sorry i know my English is not so good
  
  
  It's ok, I understand now. Those seem like good ideas.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: Codezela, davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, 
abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Hazem Salem]

Codezela added a comment.


  In D15739#356269 , @ndavis wrote:
  
  > In D15739#356026 , @Codezela 
wrote:
  >
  > > maybe there is 2 alternatives
  > >
  > > 1. remove the icon from devices section and keep it in places section 
with some icon change in the top one
  >
  >
  > You mean hide the root partition from Devices and keep the Root Places 
bookmark? That's what I currently do. I hide my `/` and `/home` partitions and 
use Places to get to those directories.
  >
  > > 2. keep it in the places section but change the label to root or system 
and it will always be top of all other hd with the lock emblem in it and remove 
the places one and we need to make the icons little bigger in this  side bar 
what do you think or i come oo late
  >
  > This confuses me. You say to keep it in Places, but remove it from Places? 
Keep what above other hard drives?
  >
  > BTW, the sidebar icons do have adjustable sizes. Perhaps it should be 
easier to find, but if you right click in an area without something selectable 
(e.g., a section label), you can go to "Icon Size" and change the size to 16, 
22, 32 or 48 px.
  
  
  i know that the icons size is adjustable i mean make the default more bigger 
first time u run dolphin this icons is so small "out of the box experience"
  the other thing  i mean we may keep the root partion icon on the devices 
sections and make it fixed position above all other mounted drives so even any 
hd mounted the root icons still top
  i hope i explain well
  sorry i know my English is not so good

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: Codezela, davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, 
abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: Codezela, davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, 
abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Noah Davis]

ndavis added a comment.


  In D15739#356026 , @Codezela wrote:
  
  > maybe there is 2 alternatives
  >
  > 1. remove the icon from devices section and keep it in places section with 
some icon change in the top one
  
  
  You mean hide the root partition from Devices and keep the Root Places 
bookmark? That's what I currently do. I hide my `/` and `/home` partitions and 
use Places to get to those directories.
  
  > 2. keep it in the places section but change the label to root or system and 
it will always be top of all other hd with the lock emblem in it and remove the 
places one and we need to make the icons little bigger in this  side bar what 
do you think or i come oo late
  
  This confuses me. You say to keep it in Places, but remove it from Places? 
Keep what above other hard drives?
  
  BTW, the sidebar icons do have adjustable sizes. Perhaps it should be easier 
to find, but if you right click in an area without something selectable (e.g., 
a section label), you can go to "Icon Size" and change the size to 16, 22, 32 
or 48 px.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: Codezela, davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, 
abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Noah Davis]

ndavis added a comment.


  In D15739#356254 , @ngraham wrote:
  
  > This argument doesn't make any sense to me. It's still there available via 
one click. I continue to fail to see how this change makes it not "easily 
accessible through Dolphin at all". You can literally get to / with a single 
click. Also, the Devices section only shows //internal// partitions so it's not 
like there will generally be 15 devices crowding the list. And the entire 
purpose of coming up with a new icon for the rood partition was to make it easy 
to pick out the root partition when there are multiple partitions in the list. 
That change was specifically requested as a precondition for doing this.
  
  
  I'm neutral on this change, but it can be argued that having Root next to all 
the other Places bookmarks makes it easier to reach. That seems rather weak 
though because it's easy to add back to the laces panel and people who might 
not want it in Places are currently unable to remove it.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: Codezela, davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, 
abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  In D15739#356010 , @davidc wrote:
  
  > I do think changing the default label of `Root`'s mounted device would 
prevent the confusion of the one user who wanted it removed from Devices.
  
  
  Okay great, I will submit a patch to improve that too.
  
  > I think the most emotional respondents consider this change to be 
condescending. Painting `/` as "too dangerous" for the typical user
  
  Who did that? I didn't. In fact, it's the UI itself that currently portrays 
Root as dangerous by displaying it with a red icon. Red == dangerous in our 
design language. If anything, this change makes browsing / //less// dangerous. 
because it's no longer represented by an icon with a red color.
  
  > Removing the quick and easy Places link and relying on users to locate 
`Root` among their mounted physical devices gives the impression that we don't 
`/` to be easily accessible through Dolphin at all, even if people can't do 
anything once they get there.
  
  This argument doesn't make any sense to me. It's still there available via 
one click. I continue to fail to see how this change makes it not "easily 
accessible through Dolphin at all". You can literally get to / with a single 
click. Also, the Devices section only shows //internal// partitions so it's not 
like there will generally be 15 devices crowding the list. And the entire 
purpose of coming up with a new icon for the rood partition was to make it easy 
to pick out the root partition when there are multiple partitions in the list. 
That change was specifically requested as a precondition for doing this.
  
  > I'm also worried this could make things more confusing for truly novice 
users, who may not realize there is a directory structure beyond `Home` at all. 
This can create many problems when trying to understand what the system is 
doing, or when troubleshooting.
  
  "Truly novice users" don't have any desire to browse their computer's 
filesystem. The only people who are ever interested in that are technical 
experts, either actual or aspiring. I don't think this change will deter them 
in any way.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: Codezela, davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, 
abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Hazem Salem]

Codezela added a comment.


  there is 2 alternatives
  
  1. remove the icon from devices section and keep it in places section with 
some icon change in the top one
  2. keep it in the places section but change the label to root or system and 
it will always be top of all other hd
  
  with the lock emblem in it and remove the places one

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: Codezela, davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, 
abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[David C]

davidc added a comment.


  Okay, I was only referring to the Places section, not the entire panel.
  
  I do think changing the default label of `Root`'s mounted device would 
prevent the confusion of the one user who wanted it removed from Devices. But 
that was only one of many criticisms of this change.
  
  I think the most emotional responses consider this change to be 
condescending. Painting `/` as "too dangerous" for the typical user, even 
though Dolphin already has restrictions that make system damage through the 
Dolphin GUI impossible. Removing the quick and easy Places link and relying on 
users to locate `Root` among their mounted physical devices gives the 
impression that we don't `/` to be easily accessible through Dolphin at all, 
even if people can't do anything once they get there.
  
  I have an additional concern that this level of obfuscation could have the 
unintended effect of making things more confusing for truly novice users, who 
may not realize there is a directory structure beyond `Home` at all. This can 
create many problems when trying to understand what the system is doing, or 
when troubleshooting problems.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, abetts, 
svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  In D15739#356008 , @davidc wrote:
  
  > Are you referring to the mounted device under the Devices section?
  
  
  Yes.
  
  > I'm unaware of how to get to the root directory in a single click using 
Places, if the existing link is removed.
  
  Perhaps a visual aid will help. Here is how the Places panel looks today, as 
of KIO's git master:
  
  F6397673: Places visual aid.png 
  
  This patch removed the red-circled item. The blue-circled item is still 
visible and still takes you to / when clicked on. Does that help? We are also 
thinking about improving the label given to the Root disk when it doesn't have 
a disk label. So instead of saying, " GiB Hard Drive", we could potentially 
make it  say "Root drive" or "Root" or " GiB Hard Drive (Root)" or anything 
else, really.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, abetts, 
svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[David C]

davidc added a comment.


  Are you referring to the mounted device under the Devices section? I'm 
unaware of how to get to the root directory in a single click using Places, if 
the existing link is removed.
  
  My question to users addressed the Devices point and I also provided the user 
feedback re: Devices. As you can see, opinion on the Devices link was mixed, 
while they were strongly in favor of the Places link. The person who said they 
wanted `Root` removed from Devices explained that the first time he used 
Plasma, he saw it mounted there and was afraid he had installed his system onto 
the wrong partition.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, abetts, 
svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  I do not understand why this change provokes any negative reactions. Right 
now, the Places panel has two items that both lead to the exact same location. 
This patch simply removes one of them by default, and adding it again if you 
want is a 10 second operation. No functionality is lost; / can still be 
accessed with a single click on an item in the Places panel by default. There 
will still be a "direct shortcut to the root directory".

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, abetts, 
svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[David C]

davidc added a comment.


  I strongly advise against this. This change goes against Plasma's brand 
identity and will provoke a strongly negative reaction from our core audiences.
  
  Plasma is strongly associated with powerful tools and high utility. Users 
also love having many options. Every time GNOME is criticized for "removing 
features," or not listening to feedback, we win big. Plasma is frequently 
praised for being the desktop environment that makes an effort to listen to its 
users.
  
  I asked for some preliminary feedback on a large Discord server for Linux 
users. I made sure to specify that the change only applied to the Places link, 
and Root would remain visible in Devices.
  
  Within **two** minutes of asking:
  
  - 4 users came out strongly against the change
  - 1 person was indifferent because they mostly used the terminal for most 
things outside of Home.
  - 2 people suggested removing the desktop link from Places instead of root.
  - 1 user asked for the removal of the Devices link to root
  - 1 person had never noticed root was listed under Devices
  
  Only the terminal user believed the Places link presented a safety risk to 
GUI users. Three people said there was no safety risk because Dolphin does not 
allow the editing of system files.
  
  Two users mentioned proprietary operating systems which provide a direct 
shortcut to the root directory.
  One user said this change would make him **"mad,"** and one person asked if 
the team thought **users were stupid.** One user said it sounded "like 
something GNOME would do, not KDE."
  
  This was a quick 5 minute survey, but the feedback was very strong and even 
emotional.
  
  My personal opinion aligns with many of these comments, and I too find this 
change out of character for Plasma.
  I contribute to Promo and I am certainly not the most technical person here. 
I use this Places link many times a day. I consider the Devices link to be 
conceptually different, and it's much less convenient because my root partition 
is in the middle of my Devices list. I don't ever feel in danger of a misclick 
because Dolphin prevents any editing or deletion of system files.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: davidc, tcanabrava, ndavis, romangg, bruns, davidedmundson, abetts, 
svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham planned changes to this revision.
ngraham added a comment.


  This blows up the places tests; will fix them in this patch.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: tcanabrava, ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, 
broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Tomaz Canabrava]

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


  too many +1 to not be accepted.

REPOSITORY
  R241 KIO

BRANCH
  remote-root-from-places-panel (branched from master)

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

To: ngraham, #dolphin, #vdg, tcanabrava
Cc: tcanabrava, ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, 
broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a dependent revision: D7446: [Places panel] Add a Recently Used 
item by default.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, broulik, 
acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham edited the summary of this revision.
ngraham added a dependency: D16653: Use the new `drive-harddisk-root` icon for 
the root volume.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, broulik, 
acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[David Edmundson]

davidedmundson resigned from this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, broulik, 
acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a subscriber: ndavis.
ngraham added a comment.


  In D15739#334513 , @romangg wrote:
  
  > Imo this patch can go in as it is, but before that I would like to see a 
patch to have the disk with the root file system getting specially marked 
(icon, color or text) for the case that several block devices are mounted at 
the same time.
  
  
  Yes, that's one of the action items for T8349: Improve Places panel usability 
and presentation . Before we can do it, we 
first needed a better disk icon, which we now have thanks to @ndavis (see 
D15853: Change drive-harddisk to more adaptable style 
)! If we want to signify root disk status 
via the icon, we still need to figure out what to do about emblems, which 
currently obscure most of the icon for the default 16x16 size. I submitted a 
patch for that, but it's run into controversy: D15866: Reduce emblem size for 
very small icons to prevent obscuring too much of the icon 
.
  
  Baby steps...

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: ndavis, romangg, bruns, davidedmundson, abetts, svenmauch, broulik, 
acrouthamel, kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Roman Gilg]

romangg added a comment.


  Imo this patch can go in as it is, but before that I would like to see a 
patch to have the disk with the root file system getting specially marked 
(icon, color or text) for the case that several block devices are mounted at 
the same time.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: romangg, bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  Thanks @bruns!  So it looks like the Windows code can go after all. 
@davidedmundson?

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Stefan Brüns]

bruns added a comment.


  In D15739#331727 , @ngraham wrote:
  
  > @bruns, do you know if Solid (and by extension, the Places panel) shows 
Windows disks when Dolphin is run on Windows?
  >
  > Side note: does anyone run Dolphin on Windows? Do we distribute it at all?
  
  
  As @davidedmundson said/hinted, Solid uses 
https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-getlogicaldrives
 to show the drives in the Devices section (same as done using e.g. UDisks2 on 
Linux), but this is independent of listing *pathes* (Root, Home) in the Places 
section.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Stefan Brüns]

bruns added a comment.


  In D15739#331700 , @davidedmundson 
wrote:
  
  > They don't.
  >
  > Why are you changing the if (windows) code path above the elif?
  
  
  Note this is `_WIN32_WCE`, i.e. dead code. WCE is no more supported by any 
half way recent Qt version.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a subscriber: bruns.
ngraham added a comment.


  @bruns, do you know if Solid (and by extension, the Places panel) shows 
Windows disks when Dolphin is run on Windows?
  
  Side note: does anyone run Dolphin on Windows? Do we distribute it at all?

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: bruns, davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham

D15739: [Places panel] Don't show Root by default

[David Edmundson]

davidedmundson added a comment.


  No idea.
  
  It isn't the same as adding root as it adds every drive quite deliberately.
  
  There is a solid windows device back end, but it needs asking someone.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  In D15739#331700 , @davidedmundson 
wrote:
  
  > They don't.
  >
  > Why are you changing the if (windows) code path above the elif?
  
  
  Because that's the windows equivalent of adding Root. I reasoned that if we 
don't want to show a Places panel entry for Root on Linux, then for similar 
reasons we wouldn't want to create Places panel entries for Windows disks when 
on Windows. Don't disks show up in the Devices section on Windows like they do 
on Linux? Can't test, sorry.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[David Edmundson]

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


  They don't.
  
  Why are you changing the if (windows) code path above the elif?

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg, davidedmundson
Cc: davidedmundson, abetts, svenmauch, broulik, acrouthamel, 
kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham added a comment.


  Do four +1s amount to any formal Accept statuses? ;-)

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, 
ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Andres Betts]

abetts added a comment.


  +1

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: abetts, svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, 
ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Sven Mauch]

svenmauch added a comment.


  All arguments make sense. It even looks a lot better without the red folder 
icon. +1

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: svenmauch, broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham, 
bruns

D15739: [Places panel] Don't show Root by default

[Kai Uwe Broulik]

broulik added a comment.


  +1 it's an entry I always hide when I setup someone's computer

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: broulik, acrouthamel, kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: acrouthamel, kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: acrouthamel, kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Andrew Crouthamel]

acrouthamel added a comment.


  +1
  
  Seeing "Root" written as such, also makes me think of `/root`, which makes no 
sense to have there. The odds of needing quick access to `/` via Dolphin are 
extremely rare, and there is already a link anyway. People are used to clicking 
the primary drive to get to root dir.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: acrouthamel, kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

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

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: ngraham, #dolphin, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15739: [Places panel] Don't show Root by default

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Dolphin, VDG.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  This patch removes Root from the default Places panel for new installations, 
for the following reasons:
  
  - It's redundant; you can still get to / on your machine in one click using 
the appropriate disk entry on the bottom of the Places panel.
  - The Root item shows up in red, indicating its danger. Dangerous items are 
probably not a good idea to show by default, especially not right next to safe 
items such as Home.
  - It's not very useful. On Linux systems, messing around with / is 
discouraged, and Dolphin doesn't even yet have the ability to modify files and 
folders there anyway even if the user wanted to; it's purely read-only. The 
usefulness of such a thing is questionable.
  - By removing it, we gain room to add something more useful such as a Recent 
Documents item (D7446 ) without making the 
Places panel show a vertical scrollbar with Dolphin's default size.

TEST PLAN
  Login in with a new user account or rm `~/.local/share/user-places.xbel` and 
open Dolphin; notice that Root is not on the Places panel.
  
  [image goes here]
  
  Existing user accounts are untouched.

REPOSITORY
  R241 KIO

BRANCH
  remote-root-from-places-panel (branched from master)

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

AFFECTED FILES
  src/filewidgets/kfileplacesmodel.cpp

To: ngraham, #dolphin, #vdg
Cc: kde-frameworks-devel, michaelh, ngraham, bruns