VSCodeVim / Vim

:star: Vim for Visual Studio Code
http://aka.ms/vscodevim
MIT License
13.52k stars 1.3k forks source link

fixes #8975 The literal string "<leader>" is substituted for the leader key when repeating a command-line command #9075

Open HenryTSZ opened 3 weeks ago

HenryTSZ commented 3 weeks ago

What this PR does / why we need it:

fixes #8975 The literal string "" is substituted for the leader key when repeating a command-line command

Which issue(s) this PR fixes

8975

Special notes for your reviewer:

The commandList here saves the ex command, which typically do not include leader.

https://github.com/VSCodeVim/Vim/blob/5365305330a69288f38acfd12ae85cba7a593fa6/src/state/recordedState.ts#L58-L59

Therefore, we do not need the following replace

https://github.com/VSCodeVim/Vim/blob/5365305330a69288f38acfd12ae85cba7a593fa6/src/state/recordedState.ts#L62-L64

J-Fields commented 1 week ago

Can you please write a test case for this?

HenryTSZ commented 6 days ago

Can you please write a test case for this?

I don't know how to write test cases. Do you have any documentation for reference?

mvandenhoek commented 4 days ago

I don't know how to write test cases. Do you have any documentation for reference?

I think the test will be similar to this one: https://github.com/VSCodeVim/Vim/blob/cfbd076c24e1497b957e7ab106ec80f46dcb60ea/test/macro.test.ts#L25-L30

To test for my issue, you'll need two such testcases. One where leader is ` (space) and one where it is` (backslash), and those keys then need to be used for a substitution command in keysPressed.

My guess is that you'll have to use the configuration to set the leader key before starting the test, see https://github.com/VSCodeVim/Vim/blob/master/src/configuration/configuration.ts

I'm not sure if for the test to simulate those key presses, the keysPressed will have to have the actual character e.g. \, or you have to write <leader>. Perhaps @J-Fields can shed some light on this?

Either way I would first check that it fails the test before your change, and then confirm that it passes after your change.

J-Fields commented 20 hours ago

I'm not sure if for the test to simulate those key presses, the keysPressed will have to have the actual character e.g. \, or you have to write <leader>. Perhaps @J-Fields can shed some light on this?

I think you have to write <leader>, but could be misremembering