[Console] Fix commands description with numeric namespaces #34130
Conversation
Why not just stick to fixing the bug? It's not strictly a bug that numeric entries come before or after alphabet entries; that's a personal preference and probably breaks someone's workflow. |
|
This bug proves that numeric namespaces were not taken into account when the code was written. Therefore it's a good occasion to improve their support in addition to purely fixing it. Displaying the numeric namespaces first is a change that makes sense to me, not because it's a personal preference but because comparing items as strings with the I doubt this is going to break someone workflow since numeric namespaces are currently not displayed correctly at all (except 0). By fixing the bug, we are already changing the output anyway. Also, I doubt we have a BC break policy on commands output. Of course, this change is a proposition that I will gladly revert if it is considered useless / bad / not backward compatible by a majority. |
I was trying not to make this about me, but the implication was that it breaks my workflow. I like my custom commands to appear at the end of the list so they don't always scroll off the top of the terminal when listing commands. Using numeric namespaces currently accomplishes this, but with your proposed change, one would have to rename everything to begin with z or some other late-sorted character to achieve the same result. |
There was a problem hiding this comment.
Let's keep the order @Bilge is used to if possible, that's 3.4, it should remain as stable as possible.
f7dc8bf to
2ef5028
Compare
|
Could you please add a test case? |
2ef5028 to
4d47868
Compare
|
I added a test. There is one edge case we don't support: when you have "0" as the command namespace, its sorted order depends of its processing order. See https://3v4l.org/SJcMB. |
|
Thank you @fancyweb. |
… (fancyweb) This PR was merged into the 3.4 branch. Discussion ---------- [Console] Fix commands description with numeric namespaces | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | #34111 | License | MIT | Doc PR | - This PR fixes the linked ticket case. It also changes the keys sorting to display the numeric namespaces first. It also fixes another bug if your command name starts with `_global:`. In this special case the command is considered global but its full name is still `_global:xxx`. We can't do better without more refactoring since the final array of namespaces and global commands is shared, `_global` just being a special key. Currently, if your command starts with `_global`, all global commands are not displayed at all so it's better like this anyway. It also fixes another bug if your command starts with `0:` (cf `'' ===` comparison). Commits ------- 4d47868 [Console] Fix commands description with numeric namespaces
This PR fixes the linked ticket case.
It also changes the keys sorting to display the numeric namespaces first.
It also fixes another bug if your command name starts with
_global:. In this special case the command is considered global but its full name is still_global:xxx. We can't do better without more refactoring since the final array of namespaces and global commands is shared,_globaljust being a special key. Currently, if your command starts with_global, all global commands are not displayed at all so it's better like this anyway.It also fixes another bug if your command starts with
0:(cf'' ===comparison).