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

fix(help): wrong full help sep rendering #554

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

luevano
Copy link
Contributor

@luevano luevano commented Jul 14, 2024

No matter the available horizontal space, sometimes the separator for the full help is not rendered. For example:

image
image

The issue is that the check if a separator needs to be rendered is done on group (the column) and not groups (the total columns). With this fixed:

image
image

However, when there is enough space for the separator but not the column itself it renders the separator at the end:
image

This PR fixes these 2 issues and matches the behavior of the short help (adding a tail if needed/possible, rendering the separator inline, etc.). Behavior now:
image
image
image

@caarlos0 caarlos0 added bug Something isn't working and removed triage labels Aug 13, 2024
@bashbunni
Copy link
Member

We should add a test for this

@bashbunni bashbunni added this to the v0.22.0 milestone Sep 19, 2024
@meowgorithm
Copy link
Member

So I think this makes sense. @luevano, do you mind providing some minimal code we can use to replicate this (in addition to a test)?

Copy link
Member

@meowgorithm meowgorithm left a comment

Choose a reason for hiding this comment

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

Alright, this is a solid fix. It just needs a test and we're good to go.

@meowgorithm
Copy link
Member

Alright, I have a test for this which I'll merge next. Thanks again for the excellent PR, and for your great patience with the one, @luevano.

@meowgorithm meowgorithm merged commit 81d444a into charmbracelet:master Oct 10, 2024
9 checks passed
@luevano
Copy link
Contributor Author

luevano commented Oct 10, 2024

Hey @meowgorithm, sorry forgot about the request for the minimal code and test, been busy with work stuff lately; thanks for keeping things moving here for me!

@meowgorithm
Copy link
Member

No worries, thanks again for the great PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants