-
Notifications
You must be signed in to change notification settings - Fork 30k
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
move builtin terminal quick fixes to contribution model #164099
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we were moving towards contributions with regexes/capture groups in package.json to help prevent perf problems?
Yes I had forgotten, but that seems better |
This reverts commit 0d97708.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to bring this to the API sync at least once before shipping it/announcing it in release notes. It could radically changes after discussing it and we don't want to waste people's time. How about we disable the extension contribution part and move the built-in ones over to the capture group system so we can essentially test the implementation of it this iteration?
src/vs/workbench/contrib/terminal/common/terminalExtensionPoints.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminal/browser/terminalQuickFixBuiltinActions.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
const actions: TerminalQuickFixAction[] = []; | ||
let fixedCommand = commandToRun; | ||
for (const [key, value] of Object.entries(groups)) { | ||
const varToResolve = '${group:' + `${key}` + '}'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can probably do '${group:' + key + '}'
since key
is already a string?
fixes #162950
fixes #163965