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

leverage Fig's shell parser, add git spec #240001

Merged
merged 55 commits into from
Feb 8, 2025
Merged

leverage Fig's shell parser, add git spec #240001

merged 55 commits into from
Feb 8, 2025

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Feb 7, 2025

fixes #239519
fixes #239518
Part of #239515

Comment on lines 424 to 425
filesRequested = true;
foldersRequested = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have the bandwidth today to think about this, but wonder if always setting these like this is now incorrect?

Copy link
Member

Choose a reason for hiding this comment

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

roblourens
roblourens previously approved these changes Feb 7, 2025
script: [
"bash",
"-c",
"git --no-optional-locks status --short | sed -ne '/^M /p' -e '/A /p'",
Copy link
Member

Choose a reason for hiding this comment

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

There are 3 cases that use | sed which will fail currently on Windows. Created #240031 to fix and upstream the change

Comment on lines 1 to 33
const AWS_SPECS = ["aws", "q"];
const UNIX_SPECS = [
"cd",
"git",
"rm",
"ls",
"cat",
"mv",
"ssh",
"cp",
"chmod",
"source",
"curl",
"make",
"mkdir",
"man",
"ln",
"grep",
"kill",
];
const EDITOR_SPECS = ["code", "nano", "vi", "vim", "nvim"];
const JS_SPECS = ["node", "npm", "npx", "yarn"];
const MACOS_SPECS = ["brew", "open"];
const OTHER_SPECS = ["docker", "python"];

export const MOST_USED_SPECS = [
...AWS_SPECS,
...UNIX_SPECS,
...EDITOR_SPECS,
...JS_SPECS,
...MACOS_SPECS,
...OTHER_SPECS,
];
Copy link
Member

Choose a reason for hiding this comment

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

This might feed into their sorting?

Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines 424 to 425
filesRequested = true;
foldersRequested = true;
Copy link
Member

Choose a reason for hiding this comment

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

@@ -44,7 +44,7 @@ export function createCodeTestSpecs(executable: string): ITestSpec[] {
{ input: `${executable} --list-extensions |`, expectedCompletions: codeSpecOptions, expectedResourceRequests: { type: 'both', cwd: testPaths.cwd } },
{ input: `${executable} --show-versions |`, expectedCompletions: codeSpecOptions, expectedResourceRequests: { type: 'both', cwd: testPaths.cwd } },
{ input: `${executable} --category |`, expectedCompletions: categoryOptions },
{ input: `${executable} --category a|`, expectedCompletions: categoryOptions.filter(c => c.startsWith('a')) },
Copy link
Member

Choose a reason for hiding this comment

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

👍

Comment on lines -73 to -74
// Filtering options should request all options so client side can filter
{ input: 'ls -a|', expectedCompletions: allOptions, expectedResourceRequests: { type: 'both', cwd: testPaths.cwd } },
Copy link
Member

Choose a reason for hiding this comment

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

If this is what fig gives us, we might need to make - a trigger character so that the list is refreshed after - and when one is deleted. #240034

Comment on lines 50 to 60
export function getEnvAsRecord(shellIntegrationEnv: { [key: string]: string | undefined } | undefined): Record<string, string> {
const env: Record<string, string> = {};
if (shellIntegrationEnv) {
for (const [key, value] of Object.entries(shellIntegrationEnv)) {
if (typeof value === 'string') {
env[key] = value;
}
}
}
return env;
}
Copy link
Member

Choose a reason for hiding this comment

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

We can maybe just do an as cast instead?

* @returns An array of `Suggestion` objects.
*
*/
postProcess?: (out: string, tokens: string[]) => (Suggestion | null)[]; // <-- VS Code edit to make results correct
Copy link
Member

Choose a reason for hiding this comment

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

@meganrogge I added these comments to the 2 lines where we diverge to we can more easily track it

Tyriar
Tyriar previously approved these changes Feb 8, 2025
@meganrogge meganrogge enabled auto-merge (squash) February 8, 2025 14:10
@meganrogge meganrogge disabled auto-merge February 8, 2025 14:21
@meganrogge meganrogge merged commit 6dbde2a into main Feb 8, 2025
8 checks passed
@meganrogge meganrogge deleted the merogge/fig-project branch February 8, 2025 15:40
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.

Leverage fig's parser implementation Terminal: Add basic subcommand support for fig specs
3 participants