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

Add support for continuation indent and optimize divide_line #3426

Closed

Conversation

noahbkim
Copy link

Type of changes

  • New feature
  • Documentation / docstrings
  • Tests

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
  • I've added tests for new code.
  • I accept that @willmcgugan may be pedantic in the code review.

Description

This PR adds an attribute continuation_indent: int to rich.text.Text that prefixes wrapped lines with a corresponding number of spaces. A paragraph wrapped to 72 characters might look like:

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim
veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea
commodo consequat.

With continuation_indent=4, it would look like:

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
    tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim
    veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex
    ea commodo consequat.

This is particularly useful for rendering multiline output that may become ambiguous without some indication of where text wrapping occurred. For example, printing a list of (very contrived) CLI commands:

echo reading file /path/to/file.txt
ssh user@host -- cat /path/to/file.txt

Might wrap to:

echo reading file
/path/to/file
ssh user@host --
cat /path/to/file

Which has a significantly different meaning. Using continuation_indent=4, we instead get:

echo reading file
    /path/to/file
ssh user@host --
    cat /path/to/file

In the process of implementing this I heavily refactored divide_line to no longer use chop_cells. Not only is this much faster (it no longer creates a temporary list of list of substrings), but it is more correct in the degenerate case of wrapping to a single column (though I'm happy to revert this if it's not preferred):

# Old behavior
>>> rich.text.Text("ab cd").wrap(rich.get_console(), 1)
Lines([<text 'a' []>, <text 'b' []>, <text ' ' []>, <text 'c' []>, <text 'd' []>])

# New behavior (no lines only containing whitespace)
>>> rich.text.Text("ab cd").wrap(rich.get_console(), 1)
Lines([<text 'a' []>, <text 'b' []>, <text 'c' []>, <text 'd' []>])

@noahbkim
Copy link
Author

@willmcgugan sorry to ping you directly but I was wondering if there was anything I should be doing to get this through review!

@willmcgugan
Copy link
Collaborator

There is a backlog that I'm slowly working through. But I will get to it.

It might help to understand a bit about why you need this. Is there precedence of this style of output?

@noahbkim
Copy link
Author

This was inspired by how PyCharm (and perhaps other modern editors) adds continuation indents when you've enable text-wrapping for documents with long lines. I'm using rich to display a report of commands run + results, and this would be particularly useful for that.

In the event you don't like continuation indents as a whole, I'd be happy to take a crack at refactoring this so I can more easily patch it myself.

@willmcgugan
Copy link
Collaborator

Sorry, I don't think I can accept this as a feature, unless I was sure there was enough support for it.

Optimizations / refactor would be fine.

https://textual.textualize.io/blog/2023/07/29/pull-requests-are-cake-or-puppies/

@noahbkim
Copy link
Author

Sorry, I don't think I can accept this as a feature, unless I was sure there was enough support for it.

No worries, thanks for taking a look!

@noahbkim noahbkim closed this Jul 26, 2024
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.

2 participants