Add option to show hidden files and folders last
AbandonedPublic

Authored by harogaston on Apr 23 2020, 3:50 AM.

Details

Reviewers
ngraham
elvisangelaccio
Group Reviewers
Dolphin
Summary

Adds a new checkbox to choose to list hidden files and folders after those not hidden. Other sorting criteria (size, date, etc) is applied afterwards.

Test Plan
  1. Open a directory with a mixture of hidden and not hidden files and folders.
  2. Select a sorting i.e. default, size, creation date, last modification date.
  3. Go to menu View -> Sort By -> Hidden Last and check the new option (by default it is unchecked). Alternatively do the same from the folder view context menu.
  4. Not hidden files should appear before hidden (or dot) files. Files and folders from each of these categories shuold still follow the sorting selection made previously.
  5. If Dolphin is configured to 'Remember properties for each folder' a new entry will appear in the '.directory' file called HiddenLast

PS: I would like to write a new test in kfileitemmodeltest.cpp but I don't know how to run the tests. Any help would be appreciated.

Diff Detail

Repository
R318 Dolphin
Branch
dot-files-last
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 25728
Build 25746: arc lint + arc unit
harogaston created this revision.Apr 23 2020, 3:50 AM
Restricted Application added a project: Dolphin. · View Herald TranscriptApr 23 2020, 3:50 AM
Restricted Application added a subscriber: kfm-devel. · View Herald Transcript
harogaston requested review of this revision.Apr 23 2020, 3:50 AM
harogaston updated this revision to Diff 80950.Apr 23 2020, 3:54 AM

Ammend previous commit. Remove unrelated change

meven added a subscriber: meven.Apr 23 2020, 7:09 AM

I am not sure we want this feature that will clutter a little more the sort popup, for such a niche use case.

Well for me it is pretty clear that this should be at least an option, if not the default. If you don't work with hidden files visible then this does nothing. But if you do, like myself, which view do you think is more useful/productive:


I open my home directory, i see only hidden folders. In most normal use cases I would go to my main folders like Documents, Downloads, Development, etc, i have to scroll through a vast number of hidden folders first.

With this option a user can still make use of having hidden files shown but also list the most likely often used entries at the beginning which only makes sense to me.

I don't think this over clutters an already long menu. But these are opinions of course.

meven added a comment.Apr 23 2020, 1:02 PM

I don't think this over clutters an already long menu. But these are opinions of course.

I would be in favor of this feature but let's wait for @ngraham and @elvisangelaccio opinions.

Code is in great shape already, since you were able to just copy paste the Folder first case ^^.

Well for me it is pretty clear that this should be at least an option, if not the default. If you don't work with hidden files visible then this does nothing. But if you do, like myself, which view do you think is more useful/productive

Right, so let's make it the default without adding an option to get a worse behavior ;)

Well for me it is pretty clear that this should be at least an option, if not the default. If you don't work with hidden files visible then this does nothing. But if you do, like myself, which view do you think is more useful/productive

Right, so let's make it the default without adding an option to get a worse behavior ;)

I think that seems sensible.

meven added a comment.Apr 27 2020, 4:16 PM

About testing, you need to install the tool ctest then go to build dir and run the whole tests with ctest (it has many options to be more clever for instance ctest -R kfileitemmodeltest

About testing, you need to install the tool ctest then go to build dir and run the whole tests with ctest (it has many options to be more clever for instance ctest -R kfileitemmodeltest

Thank you for the info. I will try to add a test then.

Well for me it is pretty clear that this should be at least an option, if not the default. If you don't work with hidden files visible then this does nothing. But if you do, like myself, which view do you think is more useful/productive

Right, so let's make it the default without adding an option to get a worse behavior ;)

So do you all want me to remove all the clutter code regarding the new setting and just keep the logic as to make it the default and only behavior?
I would rather very much keep the extra option. But it is up to you I guess.

About testing, you need to install the tool ctest then go to build dir and run the whole tests with ctest (it has many options to be more clever for instance ctest -R kfileitemmodeltest

Thank you for the info. I will try to add a test then.

Well for me it is pretty clear that this should be at least an option, if not the default. If you don't work with hidden files visible then this does nothing. But if you do, like myself, which view do you think is more useful/productive

Right, so let's make it the default without adding an option to get a worse behavior ;)

So do you all want me to remove all the clutter code regarding the new setting and just keep the logic as to make it the default and only behavior?
I would rather very much keep the extra option. But it is up to you I guess.

You said it yourself: every setting needs a good amount of clutter code, which makes the overall codebase harder to test and harder to maintain. That's why we try to avoid adding a new setting every time we add a new feature.

We can reconsider if enough people complain about this new behavior not being optional.

You said it yourself: every setting needs a good amount of clutter code, which makes the overall codebase harder to test and harder to maintain. That's why we try to avoid adding a new setting every time we add a new feature.

We can reconsider if enough people complain about this new behavior not being optional.

Like https://bugs.kde.org/show_bug.cgi?id=404955 :(

You said it yourself: every setting needs a good amount of clutter code, which makes the overall codebase harder to test and harder to maintain. That's why we try to avoid adding a new setting every time we add a new feature.

We can reconsider if enough people complain about this new behavior not being optional.

So is that a let's go with it? I'm sorry I'm kinda lost as per what you want to do exactly (if anything at all), I don't have much of a say here. My opinion I already gave.

You said it yourself: every setting needs a good amount of clutter code, which makes the overall codebase harder to test and harder to maintain. That's why we try to avoid adding a new setting every time we add a new feature.

We can reconsider if enough people complain about this new behavior not being optional.

Like https://bugs.kde.org/show_bug.cgi?id=404955 :(

That's inconvenient to say the least.

You said it yourself: every setting needs a good amount of clutter code, which makes the overall codebase harder to test and harder to maintain. That's why we try to avoid adding a new setting every time we add a new feature.

We can reconsider if enough people complain about this new behavior not being optional.

So is that a let's go with it? I'm sorry I'm kinda lost as per what you want to do exactly (if anything at all), I don't have much of a say here. My opinion I already gave.

Sorry for the delay. Yes, please remove the option and make the new behavior the default.

You may also want to move this patch to gitlab: https://invent.kde.org

You said it yourself: every setting needs a good amount of clutter code, which makes the overall codebase harder to test and harder to maintain. That's why we try to avoid adding a new setting every time we add a new feature.

We can reconsider if enough people complain about this new behavior not being optional.

So is that a let's go with it? I'm sorry I'm kinda lost as per what you want to do exactly (if anything at all), I don't have much of a say here. My opinion I already gave.

Sorry for the delay. Yes, please remove the option and make the new behavior the default.

You may also want to move this patch to gitlab: https://invent.kde.org

Hi! I'll try to look at this again. I just need to find some time. Thanks.

Hi all!

Unfortunately I'm not finding time to do this properly. I tried but my machine has changed and seems that I cannot compile and test the projects anymore.
I'll have to look into that eventually but I certainly cannot do it now.

Given the latest feedback this is all there is to change I believe:

diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp
index dbbd63a6a..fe502fbb3 100644
--- a/src/kitemviews/kfileitemmodel.cpp
+++ b/src/kitemviews/kfileitemmodel.cpp
@@ -1713,6 +1713,15 @@ bool KFileItemModel::lessThan(const ItemData* a, const ItemData* b, const QColla
         }
     }

+    // Show hidden files and folders last
+    const bool isHiddenA = a->item.isHidden();
+    const bool isHiddenB = b->item.isHidden();
+    if (isHiddenA && !isHiddenB) {
+        return false;
+    } else if (!isHiddenA && isHiddenB) {
+        return true;
+    }
+
     if (m_sortDirsFirst || m_sortRole == SizeRole) {
         const bool isDirA = a->item.isDir();
         const bool isDirB = b->item.isDir();

If anyone can push that change upstream, via Gitlab or any other means please fell free.

Sorry about the inconvenience.

Best,

Gastón

argonel added a subscriber: argonel.Feb 4 2021, 5:52 AM

Is this implementing bug 241227 (konqueror file manager: home directory should show regular folders first and then hidden folders)?

If so, I'm the person referred to in comment #1 who prefers the bash-style where the leading dot is ignored for sorting. :)

meven added a comment.Apr 27 2021, 7:23 AM

Great !

You can mark this one as abandoned.