Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Making the 'set-option' command help more descriptive. #2365

Merged
merged 4 commits into from
May 4, 2022

Conversation

heliostatic
Copy link
Contributor

@heliostatic heliostatic commented May 2, 2022

The syntax for the new set-option command is not obvious, and I had to check the code to see what it was (code comment is what is included in this PR).

@@ -1508,7 +1508,7 @@ pub const TYPABLE_COMMAND_LIST: &[TypableCommand] = &[
TypableCommand {
name: "set-option",
aliases: &["set"],
doc: "Set a config option at runtime.",
doc: "Set a config option at runtime. For example to disable smart case search, use `:set search.smart-case false`.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could add a \n in between there to format it as two lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the generated docs table doesn't seem to support new lines. I think I can use a <br> instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, can you revert that and use \n? I'm fine with it being a single line in the HTML table, I just wanted it to display as two rows in the help popup in command mode

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to, but it breaks the table rather than being one line--I'll add a screenshot shortly to show the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2022-05-02 at 14 30 47 - Marked 2 - CleanShot@2x

This is what it looks like with the `\n` in the docs.

This new help message is 18 words, 104 characters. The messages for debug-remote, primary-clipboard-yank-join, clipboard-yank-join, and write! are all longer, fwiw.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it does look better in the pop-up as two lines:
2022-05-02 at 14 35 50 - Alacritty - CleanShot@2x
vs
2022-05-02 at 14 38 18 - Alacritty - CleanShot@2x

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this is something we can fix in the cargo xtask docgen that generates the docs? We could have it turn newlines into <br>s.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushing a small patch to do this as suggested. Generates correct docs and correct in-app help:
2022-05-03 at 13 41 34 - Marked 2 - CleanShot@2x
2022-05-03 at 13 42 26 - Alacritty - CleanShot@2x

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@archseer how does this look?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 🎉

@archseer archseer merged commit 09a17e4 into helix-editor:master May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants