Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Use state of case sensitivity toggle even when find bar isn't open#7042

Merged
peterflynn merged 2 commits intomasterfrom
nj/issue-7040
Mar 1, 2014
Merged

Use state of case sensitivity toggle even when find bar isn't open#7042
peterflynn merged 2 commits intomasterfrom
nj/issue-7040

Conversation

@njx
Copy link
Copy Markdown

@njx njx commented Feb 28, 2014

@peterflynn - one line change, plus unit test

For #7040

@peterflynn
Copy link
Copy Markdown
Member

@njx Looks good, just one thought -- should we add a similar unit test for the regexp toggle? Since its state is remembered in a totally different way, might be worth testing separately...

@njx
Copy link
Copy Markdown
Author

njx commented Mar 1, 2014

Hmmm, I hadn't thought about the regexp toggle. That case seems trickier: if you do Find Next with the find bar closed, it seems like you only want to honor the regexp toggle for the last query you did in the UI itself. If you then change the query somehow (e.g. by using one of the new multiselect methods), it seems like you wouldn't want to honor the regexp toggle.

But it looks like that's actually how it works today. So I can add a unit test for that case just to verify.

(Interestingly, ST seems to get confused in this case...if you do a regexp search with the find bar open, then close it, find next works, but if you then change the query, then open the find bar again, it thinks it's in non-regexp mode even though the regexp toggle is still on.)

… bar is closed.

Also refactors out some common code from the case sensitivity test.
@njx
Copy link
Copy Markdown
Author

njx commented Mar 1, 2014

OK, new unit test added (I also slightly refactored the previous test to make them both more compact). Ready for re-review.

@njx
Copy link
Copy Markdown
Author

njx commented Mar 1, 2014

er, forgot to tag @peterflynn - ready for re-review

@peterflynn
Copy link
Copy Markdown
Member

Awesome, thanks! Merging.

peterflynn added a commit that referenced this pull request Mar 1, 2014
Use state of case sensitivity toggle even when find bar isn't open
@peterflynn peterflynn merged commit 5c420ac into master Mar 1, 2014
@peterflynn peterflynn deleted the nj/issue-7040 branch March 1, 2014 01:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants