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

Test: builtin terminal quick fixes #161846

Closed
3 tasks done
meganrogge opened this issue Sep 26, 2022 · 14 comments
Closed
3 tasks done

Test: builtin terminal quick fixes #161846

meganrogge opened this issue Sep 26, 2022 · 14 comments

Comments

@meganrogge
Copy link
Contributor

meganrogge commented Sep 26, 2022

Refs #151937

Complexity: 3

Authors: @meganrogge @Tyriar

Create Issue


Verify that the terminal quick fix experience aligns with that in the editor area.

Currently, the builtin quick fixes include:

  • git similar : git sttatus
  • git push : checkout a branch that has no upstream and git push
  • git create PR : git push --set-upstream origin <branch-name>
  • free port : launch a server or task that uses a specific <port> then launch that again so you see an error related to address already in use

Known issues:

@connor4312
Copy link
Member

Are there steps needed to enable the suggest widget? I have shell integration on with Powershell 7 on Windows, however I don't see the widget appear on the suggest misspelling, or by attempting to manually trigger via Ctrl+. like I can in the editor

@isidorn
Copy link
Contributor

isidorn commented Sep 27, 2022

Cool feature. I think this needs some polishing and will be a hit 👏
Do we have a story how to make this contributable somehow? So more and more suggestions could appear in the quick fix?

@isidorn isidorn removed their assignment Sep 27, 2022
@meganrogge
Copy link
Contributor Author

@isidorn yes we're tracking that here #145234

@meganrogge
Copy link
Contributor Author

meganrogge commented Sep 27, 2022

and have this contribution point on the terminalInstance

registerQuickFixProvider(...options: ITerminalQuickFixOptions[]): void {
for (const actionOption of options) {
this.quickFix?.registerCommandFinishedListener(actionOption);
}
}

@meganrogge
Copy link
Contributor Author

@connor4312 what input are you trying?

we have an issue tracking this that hasn't yet been completed
#161595 (comment)

@lramos15
Copy link
Member

Works well. I can't seem to find out how to get the free port suggesting to trigger though. I just use the very basic express example http://expressjs.com/en/starter/hello-world.html and I don't even see the ports panel saying the port is in use so not sure if VS Code is not detecting it.

@lramos15 lramos15 removed their assignment Sep 27, 2022
@meganrogge
Copy link
Contributor Author

@lramos15 this is a little hard to test atm because some servers don't send an exit code when the address is already in use - such as the VS Code server, so we won't trigger the suggestion.

a way to test would be to run npm start server in two terminals in the xterm.js repo

@connor4312
Copy link
Member

@connor4312 what input are you trying?

we have an issue tracking this that hasn't yet been completed #161595 (comment)

I don't think that's the same issue.

Here's a recording:

Kapture.2022-09-27.at.17.32.05.mp4

@connor4312
Copy link
Member

@meganrogge on today's Insiders which includes your PR from yesterday, I'm still not able to see the suggestion either for any git quick fixes. I also tried the server case by running node -e 'net.createServer().listen(3000)' and didn't see suggestions there either. I tried running with a fresh --user-data-dir with the same result, but decorations and "run recent command" seem to work, so shell integration overall is fine.

image

@Tyriar
Copy link
Member

Tyriar commented Sep 28, 2022

@connor4312 that won't work as it's missing an IP at the start. Created #162183 and #162175 to cover your case

@connor4312
Copy link
Member

connor4312 commented Sep 28, 2022

Ah, good finds. Though even specifying an address, I don't get anything. The I/O in that case looks like:

image

I checked that I am still on the latest Insiders:

Version: 1.72.0-insider (user setup)
Commit: 835d39cfac9ede6c7c90678c8d6c3d4d0171ed6d
Date: 2022-09-28T05:20:11.324Z
Electron: 19.0.17
Chromium: 102.0.5005.167
Node.js: 16.14.2
V8: 10.2.154.15-electron.0
OS: Windows_NT x64 10.0.22000
Sandboxed: Yes

and powershell version is also the latest stable:

PS C:\Users\connor\Github\enumerate-win32-apps> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.2.6
PSEdition                      Core
GitCommitId                    7.2.6
OS                             Microsoft Windows 10.0.22000
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

I feel like I'm doing something wrong, but am not sure what it would be... 🤔 I can be around tonight/this PST morning if you or Megan want to screenshare or collect extra info interactively

@Tyriar
Copy link
Member

Tyriar commented Sep 28, 2022

@connor4312 your stack trace was long enough that it was on the 20th line, we only scan up 20 currently so there might be an off by one error so it ends up just below the cut off?

@connor4312
Copy link
Member

connor4312 commented Sep 28, 2022

Just debugging in Insiders, it looks like the opposite: the free port matcher doesn't match anything unless the output is more than its configured 20 lines

image

If I add console.log('\r\n'.repeat(10)) then it works. THat should be enough for me to verify the UI side of this...

@Tyriar
Copy link
Member

Tyriar commented Sep 28, 2022

@connor4312 good find! Could you create an issue?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants