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: cmd+triple-click select all command output when first line wraps #5373

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dpatterbee
Copy link
Contributor

I found this bug was easily reproduced with any command that wrapped to multiple rows on the first line of its output. The cause is that we stop searching for rows once we reach the first one where row.semantic_prompt = .command, which means that we reach the bottom line of wrapped output and stop there.

This PR makes it so that we continue iterating until we reach a row where semantic_prompt != .command and then return the previous one (or the last one if we run out of rows).

I also updated the test cases to include this.

I considered that this bug would also be avoided if we didn't propagate the command semantic prompt to additional rows on wrapped lines, but I don't know enough about the shell integration to make a call on that.

Closes #4693

@dpatterbee dpatterbee changed the title Fix cmd+triple click not selecting full output fix: cmd+triple-click select all command output when first line wraps Jan 25, 2025
@dpatterbee dpatterbee marked this pull request as ready for review January 26, 2025 00:28
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

Thank you! This is looking good, just a simple request.

src/terminal/Screen.zig Show resolved Hide resolved
src/terminal/Screen.zig Outdated Show resolved Hide resolved
@jparise
Copy link
Collaborator

jparise commented Jan 26, 2025

I considered that this bug would also be avoided if we didn't propagate the command semantic prompt to additional rows on wrapped lines, but I don't know enough about the shell integration to make a call on that.

One thing that can get tricky is, in our current row-based approach, the last prompt marker is dominant, overwriting any other markers that were emitted for that row. I'm not sure if that complicates the command case, but it is relevant for multiline prompts and input regions.

I wrote some related notes in 63747d1:

Going forward, we should revisit our semantic prompt implementation. Our
row-based approach is too limiting; lines can have multiple markers, and
markers should be recorded with their full coordinates so they can form
ranges.

See: https://per.bothner.com/blog/2019/shell-integration-proposal/

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.

cmd+triple_click doesn't select full contents of the output for certain screen sizes
3 participants