Skip to content

fix escaping for terminal completions for zsh/bash/fish #251989

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

Merged
merged 8 commits into from
Jun 26, 2025

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Jun 20, 2025

fixes #249024

Screenshot 2025-06-20 at 9 28 20 AM Screenshot 2025-06-20 at 9 28 29 AM

@meganrogge meganrogge requested a review from Tyriar June 20, 2025 13:31
@meganrogge meganrogge self-assigned this Jun 20, 2025
@meganrogge meganrogge added this to the June 2025 milestone Jun 20, 2025
@meganrogge meganrogge marked this pull request as draft June 20, 2025 13:32
@meganrogge
Copy link
Contributor Author

Working on pwsh now

@meganrogge meganrogge changed the title fix escaping for bash/zsh/fish fix escaping for terminal completions Jun 20, 2025
@meganrogge
Copy link
Contributor Author

meganrogge commented Jun 20, 2025

Just realized maybe we want to handle escaping where this gets inserted instead of changing the label?

Yes, going with that instead

@meganrogge
Copy link
Contributor Author

meganrogge commented Jun 20, 2025

something weird is happening w pwsh . It gets escaped properly and sent to the terminal, but then the inserted text is wrong

Screenshot 2025-06-20 at 9 58 35 AM Screenshot 2025-06-20 at 9 58 56 AM

@meganrogge
Copy link
Contributor Author

pwsh seems involved, skipping for now

@meganrogge meganrogge changed the title fix escaping for terminal completions fix escaping for terminal completions for zsh/bash/fish Jun 20, 2025
@meganrogge meganrogge marked this pull request as ready for review June 20, 2025 14:04
@meganrogge meganrogge enabled auto-merge (squash) June 20, 2025 14:10
@Tyriar Tyriar disabled auto-merge June 20, 2025 14:15
@meganrogge
Copy link
Contributor Author

I manually tested each to make sure it's necessary @Tyriar and added tests too.

@meganrogge meganrogge enabled auto-merge (squash) June 23, 2025 19:14
@meganrogge meganrogge requested a review from Tyriar June 23, 2025 19:14
@meganrogge
Copy link
Contributor Author

@Tyriar could you pls review?

Tyriar
Tyriar previously requested changes Jun 25, 2025
@meganrogge meganrogge merged commit 1a9be6e into main Jun 26, 2025
8 checks passed
@meganrogge meganrogge deleted the scornful-hoverfly branch June 26, 2025 16:43
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.

terminal completions don't escape properly
3 participants