vimium icon indicating copy to clipboard operation
vimium copied to clipboard

Command options (in help message)

Open UncleSnail opened this issue 1 year ago • 2 comments

Description

Show command options in the help message dialogue as specified in this comment and the associated PR discussion.

Handles truncating based on the full length of the line, not just the length of the options, and shows the full options on hover.

image

Always indicates the presence of options even on lines with very long descriptions.

image

Correctly handles flags (without showing them as flag=true)

image

Handles commands with multiple options.

image

This PR also switches the new hardReload for the old reload hard to avoid repeated code/functionality.

Implementation

There could be cleaner implementations, so let me know if you have a better idea, but I believe I have found a good method that requires minimal changes and fits very well into the existing mechanism. I will describe the implementation and logic more in comments on the code.

I have formatted all new code (see #4553 for the other changes from deno fmt) and all tests pass.

UncleSnail avatar Oct 08 '24 17:10 UncleSnail

One more improvement that I think I should look into is ensuring the lines for the same command with different options inside of the help menu are reasonably ordered. Right now they aren't entirely. It's probably rare that it would be relevant, but for my setZoom commands that I use it is noticeable. image I could sort alphanumerically, but I think it makes the most sense to sort by the order the user has defined the commands in their mappings config. I plan to look into this, but I need to see if that is even feasible and it could be saved for a future PR.

UncleSnail avatar Oct 09 '24 19:10 UncleSnail

I also just pushed a commit to allow for a command + options set to be considered advanced. This way the reload hard command can be considered advanced and be hidden by default.

If you do not like this feature, I can easily remove this commit. It seems like hard reloading should be an advanced command, so I wanted to let us mark it as such, but it does add some complexity and the potential for the advanced command list to explode with advanced option choices.

Another implementation is to consider any command with options to be advanced, but then user mappings of simple commands (like new tab) with options are considered advanced and can't be seen in help without showing advanced options. I can imagine many users want to see their custom mappings without having "see advanced" checked, so I went with the implementation above. But, all options commands being advanced is a simpler solution to the problem with less scope-creep and still lets reload hard be advanced.

UncleSnail avatar Oct 09 '24 19:10 UncleSnail

@philc Let me know if there is anything you would like me to fix. Also, if all of the comment explanations/justifications are more overwhelming than helpful I can try to limit them in the future; just let me know your preference.

UncleSnail avatar Nov 19 '24 00:11 UncleSnail

@UncleSnail Will do. Thanks for posting this. I'll return to the project and review it soon!

philc avatar Nov 19 '24 05:11 philc

Excellent. Congrats on landing this!

philc avatar Jan 21 '25 01:01 philc

There were some broken tests from this PR which I couldn't figure out how to make work, and so I deleted them in 19159d75d1cda3636526053de944def2f6b051c7.

philc avatar Jan 21 '25 05:01 philc