New option "Tree view always by PID", replicating htop 2 behavior

[hishamhm]

All right, I was nerd-sniped by @fasterit into doing this :)

This implements part (2) of the comment I left here: #399 (comment)

As an end user, my 2 cents with regard to behavior would be to (1) add the auto-temporary-pause to sorted tree view, so that it matches regular view (I think this one is pretty important to avoid accidents), and (2) add Display Options toggles like [ ] Tree view is always sorted by PID which, when enabled, causes (a) entering and exiting tree view makes it return to the previously sorted list order, (b) selecting any sort order (via F6, P, M, T or clicking the header) to exit tree view. That flag would effectively restore htop 2 behavior for those of us used to it. :)

I did not implement part (1) because I'm unfamiliar with new tree-sorting logic (which I think is the most important bit, actually, but it is not as severe when the new flag is enabled — the tree only moves when processes die, which is inevitable) and I figured that (2) would be a much quicker tweak.

In order to minimize the actual feature commit, I made a refactor in the Process_compare logic, which I think is actually beneficial. I inverted the resolution of the Process_compare operation so that the "superclass" logic runs first:

• This removes duplicated code that adjusts the sort direction from every OS-specific folder.
• Most fields in a regular htop screen are OS-independent, so trying Process_compare first and only falling back to the OS-specific compareByKey function if it's an OS-specific field makes sense.
• This will allow us to override the sortKey in a global way without having to edit each OS-specific file.

And hey, in the end this PR removes more lines than it adds! 😁

I believe this fixes #399, but then a new issue could be created for "Add auto-temp-pause in sorted tree view matching behavior of list view".

🎁 Happy holidays to the htop dev team! 🎄

[BenBE]

Can you have a look how much work implementing the suggestion from #399 (comment) to use different sort order for the list mode vs. tree mode would cause? With tree mode defaulting to PID?

[BenBE]

I'm not quite happy with this patch effectively leaving us with two function callbacks for sorting in the platform structure where the first one is more or less always defaulting to a platform independent one. Can we get rid of that and just stick with the new one in its place?

[hishamhm]

Can you have a look how much work implementing the suggestion from #399 (comment) to use different sort order for the list mode vs. tree mode would cause? With tree mode defaulting to PID?

Thanks to the previous commits, it was easy! 🎉

I still kept the "Tree view always by PID" option in the Settings, which results in some specific behaviors such as "clicking on the column header to exit tree view" and "picking a new sort order to exit tree view", for the sake of the muscle memory of long time htop users. :) 🙏

I also added a minor improvement to the handling of command-line arguments so that -t and -s can be used together, now that sorted trees are a thing!

I'm not quite happy with this patch effectively leaving us with two function callbacks for sorting in the platform structure where the first one is more or less always defaulting to a platform independent one. Can we get rid of that and just stick with the new one in its place?

I don't think so, because of the order of the logic that happens: the platform-specific fallback only really needs to run if the platform-agnostic one doesn't take care of the comparison, so it makes sense that Platform_compare is the one hit by the vector sorting routine. The previous logic of redefining the object-level callback made more "OOP sense", but resulted in more repeated code and less efficient execution (or at least it seemed so by eyballing TIME+; I haven't run perf on both but I expect this to be faster). The "protocol" for a a platform-specific implementer is now simpler, in spite of the .compare = Process_compare boilerplate (which is no different than the other "Object"-level overrides). (But hey, building an OOP model in C looked like a good idea back in the day! 😆 )

[BenBE]

LGTM.

[fasterit]

I like the patch, thank you very much @hishamhm.

I'd prefer to drop the "- Tree view is always sorted by PID (htop 2 behavior)" option though as people can now select sorting for tree mode and list mode independently. That is very intuitive and provides the desired functionality.

The option is quite confusing as you cannot sort in tree mode anymore unless you disable that, so it should be "- Tree view is always sorted by PID (htop 2 behavior) which also disables sorting in tree mode so selecting sort will force you back to list mode" to really explain what it does ... I think that's overly complicated and "surprising" behavior to the user. These surprises are not desirable, UIs should be intuitive and discoverable. The two options for sort are much better from a discoverability perspective and do everything we want already.

What do you think, can we drop the configure option and the assorted logic in MainPanel.c to keep F6 behavior consistent?

[BenBE]

@fasterit The option restores previous behaviour (quick return to htop2 behaviour). For the sake of discoverability it shouldn't be default. Yet the wording might be better? Maybe - Sorting leaves tree mode (htop 2 behaviour)?