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

Enable the user to get lines from a pane via cli #2729

Closed
wants to merge 2 commits into from

Conversation

emilycares
Copy link
Contributor

This feature exists in tmux implementation and i also wanted to use it in wezterm.

@wez
Copy link
Owner

wez commented Nov 11, 2022

Thanks for this, but I don't think this will behave how you want; GetLines::lines are StableRowIndex values which are offsets from the first ever row of scrollback. When more than scrollback_lines are received, StableRowIndex = 0 will no longer be a resolvable line in the way that you want.

To work without surprises, I think you'll want to retrieve the RenderableDimensions so that you can reason about the range of lines to using either scrollback_top or physical_top as the starting index, then adding the number of lines to it. RenderableDimensions would also allow you to make it easy to select either the viewport (viewport_rows) or full scrollback (scrollback_rows) with convenient flags.

There isn't currently a way to return just RenderableDimensions in the rpc/codec stuff, so you'd need to add that functionality!

Something else that I think would be great to consider around this allowing the user to select from:

  • physical lines (what you're currently retrieving)
  • logical lines

And:

@emilycares
Copy link
Contributor Author

Thanks for looking at my pr. I have tried to implement your suggestion. The history of the output also looks fine.

@wez wez closed this in 1609fd3 Feb 5, 2023
@wez
Copy link
Owner

wez commented Feb 5, 2023

Sorry; I'd forgotten about this, and when I sat down to write up the review I figured I may was well just write the code, so I just pushed a version of wezterm cli get-text. I'd appreciate your feedback on it!

imsnif pushed a commit to imsnif/wezterm that referenced this pull request Feb 11, 2023
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