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

2019-06-24 Thread 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

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

2019-06-24 Thread Fabian Vogt
fvogt added a comment. In D15739#486043 , @bruns wrote: > In D15739#486020 , @fvogt wrote: > > > In D15739#486009 , @bruns wrote: > > > > > It actually

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

2019-06-24 Thread 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

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

2019-06-24 Thread 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

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

2019-06-24 Thread 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

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

2019-06-24 Thread Andrius da Costa Ribas
andriusr added a comment. In D15739#485978 , @fvogt wrote: > In D15739#485976 , @ngraham wrote: > > > In D15739#485975 , @fvogt wrote: > > > > > No,

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

2019-06-24 Thread 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

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

2019-06-24 Thread 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)

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

2019-06-24 Thread 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

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

2019-06-24 Thread 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

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

2019-06-24 Thread 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

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

2019-06-24 Thread 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

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

2019-06-24 Thread 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

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

2019-06-24 Thread 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

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

2019-06-23 Thread 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

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

2019-06-16 Thread 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,

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

2019-06-13 Thread 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,

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

2019-06-13 Thread 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,

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

2019-06-13 Thread 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

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

2019-06-13 Thread 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,

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

2019-06-13 Thread 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

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

2019-06-13 Thread 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,

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

2019-06-13 Thread 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

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

2019-06-13 Thread 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

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

2019-06-04 Thread 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,

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

2018-11-08 Thread 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

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

2018-11-08 Thread 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

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

2018-11-08 Thread 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:

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

2018-11-08 Thread 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

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

2018-11-08 Thread 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

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

2018-11-08 Thread Noah Davis
ndavis added a comment. In D15739#356309 , @Codezela wrote: > In D15739#356269 , @ndavis wrote: > > > In D15739#356026 , @Codezela wrote: > > > > >

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

2018-11-08 Thread 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

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

2018-11-08 Thread 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,

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

2018-11-08 Thread 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

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

2018-11-08 Thread 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

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

2018-11-08 Thread 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

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

2018-11-08 Thread 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

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

2018-11-07 Thread 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

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

2018-11-07 Thread 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

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

2018-11-07 Thread 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

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

2018-11-07 Thread 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

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

2018-11-07 Thread 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

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

2018-11-07 Thread 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,

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

2018-11-07 Thread 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:

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

2018-11-03 Thread 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,

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

2018-11-03 Thread 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,

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

2018-10-01 Thread 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

2018-10-01 Thread 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

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

2018-09-30 Thread 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

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

2018-09-25 Thread 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,

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

2018-09-25 Thread 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

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

2018-09-25 Thread 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

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

2018-09-25 Thread 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

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

2018-09-25 Thread 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,

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

2018-09-25 Thread 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

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

2018-09-25 Thread 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:

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

2018-09-25 Thread 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

2018-09-25 Thread 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

2018-09-25 Thread 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,

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

2018-09-25 Thread 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

2018-09-24 Thread 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

2018-09-24 Thread 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

2018-09-24 Thread 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

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

2018-09-24 Thread 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

2018-09-24 Thread 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

2018-09-24 Thread 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,