-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Implements vimgrep, #5991 #9630
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
base: master
Are you sure you want to change the base?
Conversation
Screen.Recording.2025-05-17.at.22.56.20.movthis is how it looks like as of now |
@J-Fields I apologize for pinging. What is the bare minimum that would be expected from a vimgrep implementation? And an off-topic question, does the VSCodeVim Slack channel still exist? I would love to ask more questions from contributors and maybe get some advice as well. Thank you in advance |
I think what you've got here is merge-able, in terms of features.
Not in any meaningful way unfortunately. At the moment I'm the only active maintainer (and that's being very loose with the term "active"). Are there are any particular questions you have? |
I had some questions about writing tests (I was confused on how to write them at all. I was wondering, for example, if it was required to use the test simplifier. I wasn't able to figure out on how to include it in my tests properly) and I was also wondering about some minor things like the command strings in the command parser. All of these got resolved now, although it's a bit saddening that the Slack is not active. Thank you for maintaining the repo, it's my favorite Vim mode implementation across different text editors and IDEs |
For cases where side-effects you care about are within the editor, it's preferred. But for something like this, where we're opening a native VSCode panel, the simplifier isn't gonna cut it.
Can you clarify what you mean?
Thanks, happy to hear it! |
I was a little confused on why the commands were parsed like 'gr' 'ep', 'fu' 'nction' etc. I tried reading the vim manual and the commands didn't look like that in there. |
My guess is that the first part in the tuple is the shorthand alias that works, i.e. 'gr' is unique so we can shorthand it and use If this is correct the I guess the entry for Also this looks like a nice way to get this feature going, nice to embrace the native VScode features sometimes. 👍 |
That's correct - this allows us to specify all the equivalent commands in one line.
Yep - I must've missed that in my first read. |
@@ -577,7 +578,8 @@ export const builtinExCommands: ReadonlyArray<[[string, string], ArgParser | und | |||
[['vert', 'ical'], undefined], | |||
[['vi', 'sual'], undefined], | |||
[['vie', 'w'], undefined], | |||
[['vim', 'grep'], undefined], | |||
[['vim', 'grep'], GrepCommand.argParser], | |||
[['grep', ''], GrepCommand.argParser], |
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.
[['grep', ''], GrepCommand.argParser], |
// This will go into the exCommandParse test | ||
// function exParseTest(input: string, parsed: ExCommand) { | ||
// test(input, () => { | ||
// const { command } = exCommandParser.tryParse(input); | ||
// assert.deepStrictEqual(command, parsed); | ||
// }); | ||
// } | ||
|
||
// suite('grep', () => { | ||
// const pattern = Pattern.parser({ direction: SearchDirection.Forward, delimiter: '/' }); | ||
// exParseTest(':vimgrep "t*st" foo.txt', | ||
// new GrepCommand({ | ||
// pattern: pattern.tryParse('t*st'), | ||
// files: ['foo.txt'], | ||
// }), | ||
// ); | ||
// }); |
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.
This should go in exCommandParse.test.ts
, I think
test('GrepCommand parses correctly', async () => { | ||
await vscode.commands.executeCommand('workbench.action.files.newUntitledFile'); | ||
const pattern = Pattern.parser({ direction: SearchDirection.Backward, delimiter: '/' }); | ||
const command = grep(pattern.tryParse('t*st'), ['Untitled-1']); | ||
assert.deepStrictEqual(command.arguments, { | ||
pattern: pattern.tryParse('t*st'), | ||
files: ['Untitled-1'], | ||
}); | ||
}); |
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.
As above, Add a few test cases to exCommandParse.test.ts
and delete this
// it will also be in normal mode if you run vimgrep from another file | ||
test('GrepCommand executes correctly', async () => { | ||
// Untitled-1 | ||
await vscode.commands.executeCommand('workbench.action.files.newUntitledFile'); |
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.
Use testUtils.createFile
instead. You can pass it the contents
directly and since it'll be an already-named file on disk, you won't have to worry about the save confirmation below.
What this PR does / why we need it:
Adds a basic implementation of vimgrep, which doesn't reinvent the wheel and uses native VSCode search function
Which issue(s) this PR fixes
#5991
Special notes for your reviewer:
I still haven't thought of how to implement the E480 error for "no match".
Also flags like
g
andj
are still missing. whilej
is implementable, I am not sure how to limit VS Code's built-in search results by a number