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.

Sign up for GitHub

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

Jump to bottom

sometimes quick fixes don't show up when they should #172068

Closed
meganrogge opened this issue Jan 23, 2023 · 28 comments · Fixed by #172471, #173615, #173872 or #175555
Closed

sometimes quick fixes don't show up when they should #172068

meganrogge opened this issue Jan 23, 2023 · 28 comments · Fixed by #172471, #173615, #173872 or #175555
Assignees
@Tyriar @meganrogge
Labels
bug Issue identified by VS Code Team member as probable bug important Issue identified as high-priority insiders-released Patch has been released in VS Code Insiders terminal-quick-fix verified Verification succeeded
Milestone
March 2023

Comments

@meganrogge
Copy link
Contributor

going to keep track of examples here to see if there's a pattern

Screenshot 2023-01-23 at 2 12 18 PM
@meganrogge meganrogge added this to the Backlog milestone Jan 23, 2023
@meganrogge meganrogge added the bug Issue identified by VS Code Team member as probable bug label Jan 23, 2023
@meganrogge
Copy link
Contributor Author

far right terminal

Screenshot 2023-01-23 at 3 07 17 PM

@meganrogge
Copy link
Contributor Author

middle terminal
Screenshot 2023-01-24 at 10 04 16 AM

@meganrogge
Copy link
Contributor Author

meganrogge commented Jan 24, 2023 •
edited

far right terminal
Screenshot 2023-01-24 at 11 45 49 AM

@meganrogge
Copy link
Contributor Author

far right terminal

Screenshot 2023-01-24 at 1 59 40 PM

@meganrogge
Copy link
Contributor Author

right terminal

Screenshot 2023-01-24 at 2 22 27 PM

@meganrogge
Copy link
Contributor Author

right terminal
Screenshot 2023-01-25 at 8 48 25 AM

@meganrogge
Copy link
Contributor Author

here are the lines when there's not a quick fix result - it contains To after remote: which we don't account for here

vscode/src/vs/workbench/contrib/terminal/browser/terminalQuickFixBuiltinActions.ts

Line 19 in 1746152

export const GitCreatePrOutputRegex = /remote:\s*(?<link>https:\/\/github\.com\/.+\/.+\/pull\/new\/.+)/;

Screenshot 2023-01-25 at 10 53 42 AM

@Tyriar
Copy link
Member

Tyriar commented Jan 25, 2023

Seeing this frequently:

image

@Tyriar Tyriar modified the milestones: Backlog, January 2023 Jan 25, 2023
@Tyriar Tyriar added the important Issue identified as high-priority label Jan 25, 2023
@meganrogge
Copy link
Contributor Author

I think we just need to include the \s*(To)*\s* to fix this

@meganrogge
Copy link
Contributor Author

or better yet, remove remote: and just look for the link

@meganrogge
Copy link
Contributor Author

meganrogge commented Jan 25, 2023 •
edited

ok I see the issue. when lines are wrapped a lot, we need to search more lines - setting the length to 10/15 fixes it even when the terminal is quite narrow

@Tyriar
Copy link
Member

Tyriar commented Jan 25, 2023

I see it on non-wrapped lines, I'm not sure I understand what you mean with the To?

@meganrogge
Copy link
Contributor Author

The to comment above was actually wrong. What's happened in the screen shot above is we abort the search early - after looking at only 5 lines. When it's actually on line 9

@meganrogge
Copy link
Contributor Author

this goes wrong when we break before reaching the content IE when line count > length

meganrogge added a commit that referenced this issue Jan 25, 2023
@meganrogge
fix #172068
895a617
@meganrogge meganrogge mentioned this issue Jan 25, 2023
Merged
meganrogge added a commit that referenced this issue Jan 25, 2023
@meganrogge
increase search area of quick fixes (#172471)
5abd226 
fix #172068
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jan 25, 2023
hediet pushed a commit that referenced this issue Jan 26, 2023
@meganrogge @hediet
increase search area of quick fixes (#172471)
a33d2bf 
fix #172068
This was referenced Feb 8, 2023
Merged
Merged
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Feb 9, 2023
c-claeys pushed a commit to c-claeys/vscode that referenced this issue Feb 16, 2023
@Tyriar @c-claeys
Fix match range of create pr quick fix
12340c9 
fixes microsoft#172068
@meganrogge
Copy link
Contributor Author

have seen the create PR not show up 3 times today. resizing doesn't cause it to show up, which means our match range is wrong/ there isn't a match

@meganrogge meganrogge reopened this Feb 17, 2023
@meganrogge meganrogge modified the milestones: February 2023, March 2023 Feb 17, 2023
@VSCodeTriageBot VSCodeTriageBot removed the insiders-released Patch has been released in VS Code Insiders label Feb 17, 2023
Tyriar added a commit that referenced this issue Feb 27, 2023
@Tyriar
Fix create pr quick fix once and for all
dc1c0c1 
The issues being fixed here is:

- The empty line in offset was not expected, the example was updated and
  4 is now used.
- The length 6 was actually resulting in a wrapped line array of length
  5, this was a bug we didn't catch when tweaking the wrapped line logic.
- A length of 6 should be sufficient, but it's upped to 12 just in case.

Fixes #172068
@Tyriar Tyriar mentioned this issue Feb 27, 2023
Merged
Tyriar added a commit that referenced this issue Feb 27, 2023
@Tyriar @meganrogge
Fix create pr quick fix once and for all
cab1324 
The issues being fixed here is:

- The empty line in offset was not expected, the example was updated and
  4 is now used.
- The length 6 was actually resulting in a wrapped line array of length
  5, this was a bug we didn't catch when tweaking the wrapped line logic.
- A length of 6 should be sufficient, but it's upped to 12 just in case.

Fixes #172068

Co-authored-by: Megan Rogge <merogge@microsoft.com>
@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Feb 27, 2023
@meganrogge
Copy link
Contributor Author

this says unreleased, but was merged before the current version of insider's (5 hours ago) was released. create with PR still failing

Screenshot 2023-02-28 at 10 56 19 AM

@meganrogge meganrogge reopened this Feb 28, 2023
@meganrogge meganrogge removed the unreleased Patch has not yet been released in VS Code Insiders label Feb 28, 2023
@meganrogge meganrogge added the unreleased Patch has not yet been released in VS Code Insiders label Feb 28, 2023
@meganrogge
Copy link
Contributor Author

insider's is based on the release branch, so I was confused

@VSCodeTriageBot VSCodeTriageBot removed the unreleased Patch has not yet been released in VS Code Insiders label Mar 2, 2023
@VSCodeTriageBot
Copy link
Collaborator

Issue marked as unreleased but unable to locate closing commit in issue timeline. You can manually reference a commit by commenting \closedWith someCommitSha, or directly add the insiders-released label if you know this has already been releaased

@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label Mar 2, 2023
@meganrogge meganrogge added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Mar 2, 2023
@amunger amunger added the verified Verification succeeded label Mar 22, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Apr 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Tyriar Tyriar

@meganrogge meganrogge

Labels
bug Issue identified by VS Code Team member as probable bug important Issue identified as high-priority insiders-released Patch has been released in VS Code Insiders terminal-quick-fix verified Verification succeeded
Projects
None yet
Milestone
March 2023
6 participants
@amunger @Tyriar @rzhao271 @bhavyaus @meganrogge @VSCodeTriageBot