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

Incsearch support for :global #894

Merged
merged 36 commits into from
Jun 3, 2024

Conversation

citizenmatt
Copy link
Member

Well this PR is a great example of yak shaving. I wanted to modify one line in the ex text field change listener to add instanceof GlobalCommand so it would update highlights for the :global command, and, well, this PR happened instead. The big problem is that :global has a default range of the whole file, where (nearly) all other commands have a default range of the current line, and that wasn't easily added.

So, this PR refactors range handling to allow the :global command to have a different default range to the other commands. It improves and simplifies the API and uses the same terminology as Vim - a "range" is a collection of "addresses". An address evaluates to a line number, and can be an explicit number, such as 1 or 2, or a string like . for the current line, $ for the last line, 'a for a mark, etc., or even a search pattern. Addresses can also have +/- offsets. The range % is a collection of the first line to the last line in a file.

It also fixes a number of off-by-one errors; addresses are one-based, while IdeaVim internally is zero-based. This is particularly important because the address 0 is special. For the :[range]move {address} or :[range]copy {address} command, a value of 1 for {address} means move/copy to the line after the first line, while 0 means move/copy to the line before the first line. This {address} should not be confused with {count} in :[range]yank {count}, which is also 1-based, but should be treated as 1-based by IdeaVim.

As part of the refactoring, it fixes the following issues:

  • Fixes off-by-one error in :[count]goto to go to the [count]th character in a file
  • Fixes :[range]copy 0 to correctly copy to the line before the first line
  • Fixes :>{count} and :<{count}
  • Fixes correct end location of caret after various ex operations
  • Fixes maintaining correct mode (i.e. Visual) when cancelling search entry
  • Fixes clearing the output panel after :global command (VIM-2570)
  • Fixes range containing 0 not being handled correctly, e.g. :0,$s/... (VIM-1137)
  • Fixes range with missing last address, e.g. :1,d (VIM-992)
  • Fixes incorrect value returned from line() script function when in Normal mode
  • Fixes :[range]m. move to current line (VIM-2936)
  • Reports correct Vim errors for invalid range, mark not set, etc.
  • Add more tests and minor fixes for range, count, address and register in commands such as :copy, :move, :goto (goto character), :[range] (goto line), :delete, :join, :yank

Finally, of course, it adds 'incsearch' highlighting for the :global command (VIM-2891).

An address evaluates to a line, and a range is a collection of addresses
Sometimes we cache things, and other times it's relative to a passed caret. Let's always calculate it
Remove mutation from LineAddress
Provide caret when calling from Command
Also ensure that test derives from VimTestCase so that injector is correctly initialised
Count needs to be one-based, lines must be zero-based. So store addresses as one-based until processed
Also start to refactor handling of count
Correctly handles some validation, and also allows going to line zero
Validates register before use and correctly uses register and count
Made it explicit to get the count from argument and/or range. Default count is not passed, because it was never used. Added some tests where possible, but hard to test select file and friends
Shift left and right now work with counts, validate the counts and move the caret to the correct end position
Handles validation of count and correctly moves caret to end of range after execution. Also fix issue where the results of :print are accumulated and not cleared.

Fixes VIM-2570
Handles validation for count and positions caret in the correct place. Also handles join with visual multicaret scenarios.
Handles validation for count and ensures correct behaviour for registers.
Specify a default range instead of default line for count.
This ensures that a failing action doesn't pass due to previous state
Removes a workaround that would break moving a range to the current line because it would always move the caret to the start of the range. Now positions the caret to the start of the selection if there is one. This also means we can remove the SAVE_VISUAL flag from JoinLinesCommand

Fixes VIM-2936
We need to share the implementation between starting an Ex command, and starting a filter command, which is just an Ex command with initial text
Commands are executed in Normal mode, although there may be multiple carets
Fixes regression from changes in ex field handling
E.g. `2"a3"b4"c5d6/foo` will now highlight the current match correctly
@citizenmatt
Copy link
Member Author

Rebased and force pushed. There were some conflicts with recent changes to handling the ex text field, so the last 7 commits should be reviewed again. The conflicts highlighted a bug and a regression, which are also fixed in these last commits.

@AlexPl292 AlexPl292 merged commit d00bd8b into JetBrains:master Jun 3, 2024
4 checks passed
@AlexPl292
Copy link
Member

Merged! Thank you!

@citizenmatt citizenmatt deleted the bugfix/incsearch-for-global branch June 3, 2024 09:18
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.

2 participants