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

Disable markdown styling in --help #450

Closed
1st1 opened this issue Aug 8, 2021 · 7 comments · Fixed by #453
Closed

Disable markdown styling in --help #450

1st1 opened this issue Aug 8, 2021 · 7 comments · Fixed by #453
Assignees

Comments

@1st1
Copy link
Member

1st1 commented Aug 8, 2021

It looks like markdown styling causes some artifacts in terminals. Most likely because text wrap happens after the escape codes for formatting are added, but wrapping and styling properly at the same time sounds like a complicated enough problem. So I propose simply disabling styling of markdown for now in terminal.

Screenshot from 2021-08-07 11-21-12

@tailhook tailhook self-assigned this Aug 9, 2021
1st1 added a commit that referenced this issue Aug 9, 2021
The reason is unwanted rendering artifacts in some terminals causes by
clap wrapping the text after markdown parsing/styling is done. Perhaps
we can figure out how to actually fix this rendering issue in a better
way, but until then it's safer to just disable the feature.

Closes #450.
@tailhook
Copy link
Contributor

tailhook commented Aug 9, 2021

Well, actually word-wrapping by markdown works fine. So the simplest solution would be to fix width of the formatted text. We already have it in the quite narrow range of 50..90. So I'm in doubt that there will be any usability issues if we just fix the width to 90 or 80.

@1st1
Copy link
Member Author

1st1 commented Aug 9, 2021

The artifacts still be there if the terminal window is of certain size. Also keeping the current formatting makes it easy to break rendering by accident, just by rearranging the text. Also I don't see a lot of value in highlighting text in ... -- moreover, that will look weird if the terminal background is white.

The motivation to migrate to markdown was not to highlight it in --help, but rather to later generate man pages and render html docs.

@tailhook
Copy link
Contributor

tailhook commented Aug 9, 2021

The artifacts still be there if the terminal window is of certain size.

Rewrapping by terminal emulator or tmux doesn't cause any artifacts (except text at the left, which is expected, and will be in your solution too).

We also can rewrap markdown on the fly, just have to move markdown rendering code from codegen to generated function, which is a little bit more work, but not a lot.

The motivation to migrate to markdown was not to highlight it in --help, but rather to later generate man pages and render html docs.

Sure. But stripping markdown by regexes is not a great solution too. If you don't like styles, please, adjust styles. But markdown should be parsed as markdown.

@1st1
Copy link
Member Author

1st1 commented Aug 9, 2021

Rewrapping by terminal emulator or tmux doesn't cause any artifacts (except text at the left, which is expected, and will be in your solution too).

Not sure I follow.

We also can rewrap markdown on the fly, just have to move markdown rendering code from codegen to generated function, which is a little bit more work, but not a lot.

Let's do that, I'm not opposed to it.

Sure. But stripping markdown by regexes is not a great solution too. If you don't like styles, please, adjust styles. But markdown should be parsed as markdown.

Paul, first, I'd like to fix a bug we currently have. If you have a better fix - please propose it, but the fix should be better and not just about disabling formatting tailored to term width.

Second, the current styling is problematic and I'm not sure how to fix it. Currently, text in backticks renders with a light-grey background, without backticks. So if the user's terminal background is configured to be a shade of grey, they will not see the neither the highlighting nor backticks, making the help text very hard to parse. I guess the only unambiguous styling here would be to make text in backticks bold.

In any case, if you're against the proposed PR - np, please make a better one. But I'm -1 on just fixing the width of the output just so that it happens to work with the current text.

@1st1
Copy link
Member Author

1st1 commented Aug 9, 2021

We also can rewrap markdown on the fly, just have to move markdown rendering code from codegen to generated function, which is a little bit more work, but not a lot.

I've tested my patch for different terminal width > 50 characters, the text reflows correctly all the time.

1st1 added a commit that referenced this issue Aug 9, 2021
While there, colorize the "about" section of `--help`.

Fixes #450; superseedes #452.
1st1 added a commit that referenced this issue Aug 10, 2021
While there, colorize the "about" section of `--help`.

Fixes #450; superseedes #452.
@tailhook
Copy link
Contributor

tailhook commented Aug 10, 2021

I've tested my patch for different terminal width > 50 characters, the text reflows correctly all the time.

I'm not saying your patch reflows incorrectly. I'm saying that some markdown markup that is not converted to a proper formatting may look ugly. I'm not sure we use anything except backticks now, but I think this is ugly thing go remember.
Update: If you are describing your new PR, it's fine.

@1st1
Copy link
Member Author

1st1 commented Aug 10, 2021

Yeah, it took me some time to realize that my first patch would render any multiline code block horribly wrong (although we're not using any right now, it's still too fragile)

@1st1 1st1 closed this as completed in #453 Aug 10, 2021
1st1 added a commit that referenced this issue Aug 10, 2021
While there, colorize the "about" section of `--help`.

Fixes #450; superseedes #452.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants