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.

Sign up for GitHub

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

bug: fixes behaviour of search when using * and # (fixes #2517) #2518

Merged
merged 5 commits into from
Apr 10, 2018
Merged

bug: fixes behaviour of search when using * and # (fixes #2517) #2518

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4037874
An attempt to implement #2517
clamb Apr 6, 2018
cbb94fe
Uses VimError instead of Error when no search pattern is provided to …
clamb Apr 9, 2018
195d190
Remove E476 that is not being used.
clamb Apr 9, 2018
8badb96
Remove useless comment.
clamb Apr 9, 2018
4aa7dbe
Merge branch 'master' into master
clamb Apr 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions src/cmd_line/commands/substitute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,16 @@ export class SubstituteCommand extends node.CommandBase {
jsRegexFlags += 'i';
}

// If no pattern is entered, use previous search state
// If no pattern is entered, use previous search state (including search with * and #)
if (args.pattern === '') {
const prevSearchState = vimState.globalState.searchStatePrevious;
if (prevSearchState) {
const prevSearchString = prevSearchState[prevSearchState.length - 1].searchString;
args.pattern = prevSearchString;
const prevSearchState = vimState.globalState.searchState;
if (prevSearchState === undefined || prevSearchState.searchString === '') {
// Should we use VimError (in src/error.ts) ?
throw new Error('Empty search string!');
Copy link
Member

Choose a reason for hiding this comment

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

Can you implement and throw the VimError?

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 tried, but the handling of VimError is either broken or, more likely, I don't understand how it works.

Whenever a VimError is thrown, StatusBarImpl.setText is invoked with the text given to VimError (commandLine.ts), but is then immediately invoked again with "-- NORMAL -- :" and again with "-- NORMAL -- " as text. Therefore, the user does not get a chance to see the error thrown by VimError.

animated

Copy link
Member

Choose a reason for hiding this comment

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

That looks to be a separate bug with how we are handling thrown VimErrors. We can either fix it in your PR here, or considering there's no behaviour change, we can merge your PR and file a subsequent issue to fix it later

} else {
args.pattern = prevSearchState.searchString;
}
}

return new RegExp(args.pattern, jsRegexFlags);
}

Expand Down
128 changes: 128 additions & 0 deletions test/cmd_line/substitute.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,4 +212,132 @@ suite('Basic substitute', () => {
assertEqualLines(['bz']);
});
});
suite('Substitute with empty search string should use previous search', () => {
test('Substitute with previous search using *', async () => {
await modeHandler.handleMultipleKeyEvents([
'i',
'f',
'o',
'o',
'<Esc>',
'o',
'b',
'a',
'r',
'<Esc>',
'o',
'f',
'o',
'o',
'<Esc>',
'o',
'b',
'a',
'r',
'<Esc>',
'g',
'g', // back to the first line
'*', // search for foo
]);
await CommandLine.Run('%s//fighters', modeHandler.vimState);

assertEqualLines(['fighters', 'bar', 'fighters', 'bar']);
});
test('Substitute with previous search using #', async () => {
await modeHandler.handleMultipleKeyEvents([
'i',
'f',
'o',
'o',
'<Esc>',
'o',
'b',
'a',
'r',
'<Esc>',
'o',
'f',
'o',
'o',
'<Esc>',
'o',
'b',
'a',
'r',
'<Esc>',
'#', // search for bar
]);
await CommandLine.Run('%s//fighters', modeHandler.vimState);

assertEqualLines(['foo', 'fighters', 'foo', 'fighters']);
});
test('Substitute with previous search using /', async () => {
await modeHandler.handleMultipleKeyEvents([
'i',
'f',
'o',
'o',
'<Esc>',
'o',
'b',
'a',
'r',
'<Esc>',
'o',
'f',
'o',
'o',
'<Esc>',
'o',
'b',
'a',
'r',
'<Esc>',
'/',
'f',
'o',
'o', // search for foo
'\n',
]);
await CommandLine.Run('%s//fighters', modeHandler.vimState);

assertEqualLines(['fighters', 'bar', 'fighters', 'bar']);
});
test('Substitute with empty search string should use last searched pattern', async () => {
await modeHandler.handleMultipleKeyEvents([
'i',
'f',
'o',
'o',
'<Esc>',
'o',
'b',
'a',
'r',
'<Esc>',
'o',
'f',
'o',
'o',
'<Esc>',
'o',
'b',
'a',
'r',
'<Esc>',
'/',
'f',
'o',
'o', // search for foo
'\n',
'2', // go to the second line
'g',
'g',
'*', // now search for bar
]);
await CommandLine.Run('%s//fighters', modeHandler.vimState);

assertEqualLines(['foo', 'fighters', 'foo', 'fighters']);
});
});
});