Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Console] Fix commands description with numeric namespaces#34130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Console] Fix commands description with numeric namespaces#34130
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Bilge commentedOct 31, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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. |
fancyweb commentedOct 31, 2019
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. |
Bilge commentedNov 1, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I was trying not to make this about me, but the implication was that it breaksmy 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 withz or some other late-sorted character to achieve the same result. |
nicolas-grekas left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Let's keep the order@Bilge is used to if possible, that's 3.4, it should remain as stable as possible.
Uh oh!
There was an error while loading.Please reload this page.
f7dc8bf to2ef5028Comparenicolas-grekas commentedNov 28, 2019
Could you please add a test case? |
2ef5028 to4d47868Comparefancyweb commentedNov 28, 2019
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. Seehttps://3v4l.org/SJcMB. |
nicolas-grekas commentedNov 28, 2019
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
Uh oh!
There was an error while loading.Please reload this page.
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).