Global command implementation #9769

martchellop · 2025-09-27T22:57:05Z

What this PR does / why we need it:

This PR implements the :g[lobal]/[pattern]/[command] feature

Which issue(s) this PR fixes

#3010

Special notes for your reviewer:

This is my first time in this code base and I used Kiro. All the tests are passing, but review it carefully.

Merge

benatagirre · 2025-10-27T11:55:54Z

I see this also implements the :v command. I've always missed that one.

J-Fields · 2025-12-20T00:09:16Z

src/cmd_line/commands/global.ts

+
+  private static createParser(inverse: boolean): Parser<GlobalCommand> {
+    return optWhitespace.then(
+      regexp(/[^\w\s\\|"]{1}/)


{1}... doesn't that mean "1 time"? Is that necessary?

J-Fields · 2025-12-20T00:15:55Z

src/cmd_line/commands/global.ts

+    const position = new Position(lineNumber, 0);
+    vimState.cursorStopPosition = position;
+    vimState.cursorStartPosition = position;


Suggested change

const position = new Position(lineNumber, 0);

vimState.cursorStopPosition = position;

vimState.cursorStartPosition = position;

vimState.cursor = Cursor.atPosition(new Position(lineNumber, 0));

J-Fields · 2025-12-20T00:17:29Z

src/cmd_line/commands/global.ts

+    const position = new Position(lineNumber, 0);
+    vimState.cursorStopPosition = position;
+    vimState.cursorStartPosition = position;
+    vimState.editor.selection = new vscode.Selection(position, position);


Is setting the selection here necessary? Whenever possible we try to just set the cursor(s) on VimState then let ModeHandler.updateView() set the editor selections accordingly.

(here and elsewhere)

J-Fields · 2025-12-20T00:18:01Z

src/cmd_line/commands/global.ts

+      StatusBar.displayError(
+        vimState,
+        VimError.fromCode(ErrorCode.NotAnEditorCommand, commandToExecute),
+      );
+      throw new Error('Command parsing failed');


Why not just throw the VimError?

J-Fields · 2025-12-20T00:19:34Z

src/cmd_line/commands/global.ts

+    vimState.cursorStopPosition = finalPosition;
+    vimState.cursorStartPosition = finalPosition;


This is redundant if we are going to do vimState.cursors = [new Cursor(finalPosition, finalPosition)]; two lines below

J-Fields · 2025-12-20T00:20:07Z

src/cmd_line/commands/global.ts

+    vimState.cursorStartPosition = finalPosition;
+    vimState.editor.selection = new vscode.Selection(finalPosition, finalPosition);
+    vimState.cursors = [new Cursor(finalPosition, finalPosition)];
+    vimState.lastVisualSelection = undefined;


J-Fields · 2025-12-20T00:22:20Z

src/cmd_line/commands/global.ts

+   * Updates line tracking based on the executed command and its effects on the document
+   */
+  private updateLineTracking(
+    parsedCommand: ExCommand | null,


Is this ever null?

J-Fields · 2025-12-20T00:23:00Z

src/cmd_line/commands/global.ts

+    } else if (parsedCommand instanceof SubstituteCommand || commandString.startsWith('s')) {
+      // Substitute commands: can add or remove lines based on replacement text
+      executionContext.lineOffset += lineCountChange;
+    } else if (commandString.startsWith('t') || commandString.startsWith('co')) {


Why not parsedCommand instanceof CopyCommand like above?

J-Fields · 2025-12-20T00:23:49Z

src/cmd_line/commands/global.ts

+    executionContext: ExecutionContext,
+  ): void {
+    // Handle different command types and their line count effects
+    if (parsedCommand instanceof DeleteCommand || commandString.startsWith('d')) {


Why are we bothering with commandString after we parsed the command?

J-Fields · 2025-12-20T00:24:49Z

src/cmd_line/commands/global.ts

+  constructor(args: IGlobalCommandArguments) {
+    super();
+
+    // Validate pattern is not empty (Requirement 6.1)


I assume Requirement 6.1 refers to an AI prompt?

Copilot

Pull request overview

This PR implements the :g[lobal]/[pattern]/[command] feature for VSCodeVim, enabling users to execute commands on lines matching (or not matching) a pattern across the entire document or a specified range. This is a fundamental Vim feature that has been requested in issue #3010.

Key Changes:

New GlobalCommand class supporting :g, :global, :g!, :v, and :vglobal variants
Comprehensive line tracking to handle document modifications during command execution
Atomic undo/redo support treating the entire global operation as a single change
Pattern-based line selection with support for multiple delimiter characters

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/cmd_line/commands/global.ts	New implementation of the global command with pattern matching, line tracking, and command execution on matching lines
src/vimscript/exCommandParser.ts	Registration of global command variants (g, global, g!, v, vglobal) with their respective parsers
test/cmd_line/global.test.ts	Comprehensive test suite covering parser variants, pattern delimiters, command execution, ranges, error handling, undo/redo, and edge cases
test/vimscript/exCommandParse.test.ts	Parser tests for global command syntax validation
.gitignore	Added .kiro directory to ignore list for AI assistant artifacts

src/vimscript/exCommandParser.ts

test/vimscript/exCommandParse.test.ts

Copilot · 2025-12-20T00:30:43Z

src/cmd_line/commands/global.ts

+    // Validate pattern is not empty (Requirement 6.1)
+    if (!args.pattern || args.pattern.patternString === '') {
+      throw VimError.fromCode(ErrorCode.NoPreviousRegularExpression);
+    }
+


The constructor incorrectly rejects empty patterns. In Vim, when using :g//command with an empty pattern, it should use the last search pattern from globalState.searchState, not throw an error. Similar to how SubstituteCommand handles empty patterns (see substitute.ts lines 531-542), this should check if the pattern is empty and fall back to the previous search state. Currently, this makes the test on line 292-298 of global.test.ts fail to work as expected.

Suggested change

// Validate pattern is not empty (Requirement 6.1)

if (!args.pattern || args.pattern.patternString === '') {

throw VimError.fromCode(ErrorCode.NoPreviousRegularExpression);

}

src/cmd_line/commands/global.ts

Copilot · 2025-12-20T00:30:44Z

src/cmd_line/commands/global.ts

+
+  private handleCommandError(
+    vimState: VimState,
+    error: any,


The error parameter type is any, which loses type safety. Consider using a more specific type such as unknown and then performing type checking within the function, or Error | VimError if those are the expected error types. Using any bypasses TypeScript's type checking.

Suggested change

error: any,

error: unknown,

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

martchellop and others added 14 commits August 16, 2025 17:52

Merge pull request #1 from VSCodeVim/master

2ce3b19

Merge

Adds GlobalCommand with placeholder implementation

09bce99

Implement argument parser for global command

6b2ccb5

Register global command variants in command parser

d3a66bc

Implement logic

ad70f1d

Implement error handling and validation

be11595

Implement undo

1d5ba78

Add VSCode integration features

2bf4608

Add test suite

72ccc9d

Add error handling and edge case tests

05e586e

Add integration and undo tests

dfda4d9

Refactors parser and varg parser into shared method

e797e11

Refactor GlobalCommand to make it more readable

3965e54

Merge branch 'master' into global-command

eedb382

J-Fields mentioned this pull request Dec 20, 2025

Issue resolution: :g[lobal] feature not present #9556 #9557

Closed

Merge branch 'master' into global-command

6ffa1cf

J-Fields reviewed Dec 20, 2025

View reviewed changes

J-Fields requested a review from Copilot December 20, 2025 00:25

Copilot started reviewing on behalf of J-Fields December 20, 2025 00:25 View session

Copilot AI reviewed Dec 20, 2025

View reviewed changes

J-Fields and others added 2 commits December 19, 2025 19:31

Apply suggestion from @Copilot

e7b45cc

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Apply suggestion from @Copilot

e206fc7

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

		vimState.cursorStopPosition = finalPosition;
		vimState.cursorStartPosition = finalPosition;

Uh oh!

Global command implementation #9769

Are you sure you want to change the base?

Global command implementation #9769

Conversation

martchellop commented Sep 27, 2025

Uh oh!

benatagirre commented Oct 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 20, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants