Skip to content

Conversation

@meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Apr 24, 2025

fixes #211857

Previously, both the Create PR and Push Upstream terminal quick fixes required the command line to match a specific regex:

However, in certain cases, the captured commandLine was incorrectly set to gitpush instead of git push. This occurred when the user entered git and push on separate lines — such as when a command wrapped due to line length.

Wrapped command

During the execute phase, the command was correctly recognized. But by the finished phase, the space had been lost, breaking the match:

Incorrect by finished

The issue was caused by trimming whitespace before combining wrapped lines — which removed the space between command parts.

Daniel suggested instead of changing that logic, we replace it and use the promptInputModel.value instead which does not have this issue.

Working case

@meganrogge meganrogge requested a review from Tyriar April 24, 2025 21:10
@meganrogge meganrogge self-assigned this Apr 24, 2025
@meganrogge meganrogge added this to the June 2025 milestone Apr 24, 2025
@meganrogge meganrogge changed the title fix longstanding, annoying terminal quick fix bugs Fix terminal quick fix bugs Apr 24, 2025
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

The trim there was intentional I believe to combating potential whitespace cells that are left around from previous renders on that line. For example if when you backspace on a shell, it prints the space character instead of actually clearing it.

What this is doing is very similar to prompt input model, I wonder if we could leverage that logic here instead of doing it manually? PromptInputModel is owned by CommandDetectionCapability, so it might just be a matter of using PromptInputModel.value instead (removing ghost text is ghostTextIndex is !== -1).

@meganrogge meganrogge requested a review from Tyriar April 25, 2025 15:26
@meganrogge meganrogge modified the milestones: June 2025, April 2025 Apr 25, 2025
@meganrogge meganrogge enabled auto-merge (squash) April 25, 2025 15:26
@meganrogge meganrogge modified the milestones: April 2025, May 2025 Apr 26, 2025
@meganrogge meganrogge merged commit 317d89d into main Apr 29, 2025
8 checks passed
@meganrogge meganrogge deleted the merogge/fix-bug-quick-fix branch April 29, 2025 17:58
@meganrogge
Copy link
Contributor Author

This was supposed to be merged in May... my bad for leaving auto-merge enabled on it. It's probably fine.

@meganrogge meganrogge modified the milestones: May 2025, April 2025 Apr 29, 2025
NikolaRHristov pushed a commit to CodeEditorLand/Editor that referenced this pull request Apr 30, 2025
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Jun 13, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sometimes quick fixes don't work in the terminal on macOS

4 participants