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(viewport): properly truncate to size #228

Merged
merged 2 commits into from Sep 2, 2022

Conversation

knz
Copy link
Contributor

@knz knz commented Aug 31, 2022

Fixes #227

  • logical content is wider than display width (long lines).
    In this case, we truncate.

  • logical content is smaller than display height (fewer lines
    than visible).
    In this case, we pad with empty lines up to the specified height.

  • logical content is higher than display height (more lines than
    visible).
    In this case, we truncate.

  • style specifies a width wider than the display width.
    In this case, we ignore the style width and
    fit in the display width.

  • style specifies a height higher than the display height.
    Same as width, we ignore the style and fit in the display height.

  • style specifies a narrower width or smaller height than the display.
    In this case we obey the style.

cc @meowgorithm

@meowgorithm
Copy link
Member

meowgorithm commented Aug 31, 2022

Thanks for this @knz. One thing I'm noticing is that borders, margins, and padding don't seem to accounted for at render time (see example). Otherwise, everything seems good.

@knz knz mentioned this pull request Sep 1, 2022
34 tasks
@knz
Copy link
Contributor Author

knz commented Sep 1, 2022

Ok I've updated it.

But note that your example here sets a width in the style, which none of the bubbles really support well (I'm fixing viewport in this PR, but the other bubbles will remain weird in that case).

This of course would benefit from unit tests. I'll send a separate PR for that.

There are many "interesting" cases:

- logical content is wider than display width (long lines).
  In this case, we truncate.
- logical content is smaller than display height (fewer lines
  than visible).
  In this case, we pad with empty lines up to the specified height.
- logical content is higher than display height (more lines than
  visible).
  In this case, we truncate.

- style specifies a width wider than the display width.
  In this case, we ignore the style width and
  fit in the display width.
- style specifies a height higher than the display height.
  Same as width, we ignore the style and fit in the display height.
- style specifies a narrower width or smaller height than the display.
  In this case we obey the style.
@meowgorithm
Copy link
Member

Nicely done, @knz. I've played with it a bit and it seems perfect now.

But note that your example here sets a width in the style, which none of the bubbles really support well (I'm fixing viewport in this PR, but the other bubbles will remain weird in that case).

If you're referring to this part, it's only for text wrapping, which I imagine is an opt-in feature we should add at some point.

@meowgorithm meowgorithm merged commit 278edd1 into charmbracelet:master Sep 2, 2022
@knz knz deleted the 20220830-ta branch September 3, 2022 09:25
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.

viewport: width and height are not respected if input contains long lines
2 participants