proposal to fix issue 21600

[wimspaargaren]

Hello, I got a proposal to resolve #21600.

The showPreviousSearchTerm function tried to get the previous search term, but since clear() does not add a value to the searchHistory the showPreviousSearchTerm returned the one last search term.

This is my first pull request, so please let me know if anything is wrong.

Hope it helps!

[mention-bot]

@wimspaargaren, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sandy081 and @jrieken to be potential reviewers.

[msftclas]

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

[sandy081]

@wimspaargaren Thanks for the PR. I will take a look and get back to you. Before that as a first step, can you please make sure that (following) builds are green and also have the proper formatting.

[wimspaargaren]

@sandy081 Yes I will look into it now.

[wimspaargaren]

@sandy081 I fixed the formatting issues.

[wimspaargaren]

@sandy081 I found that my previous solution introduced a new bug, so I created a simpler, and I think also better, solution to fix the bug. Let me know if I should adjust anything.

[sandy081]

@wimspaargaren New approach looks good.. but this approach made me think about another related case.. what if user types some text after clearing and try to get the previous term (using ctrl + up) ?

How about adding the current search term to the history, when user asks for previous term ?

[wimspaargaren]

@sandy081 if I understand you correctly you want the following:

If a user enters a search term, doesn't hit enter, but presses ctrl+up instead, the current search term needs to be added to the history.

I will look further into this tomorrow afternoon and let you know when I came up with a solution.

[wimspaargaren]

@sandy081 I couldn't resist to implement it already :) Is this what you meant, and if so do you think this is a good solution?
I added an extra function to the HistoryNavigator called addIfNotPresent, which checks if a HistoryElement is already present in the history. When using the normal add, showPreviousTerm(alt + up) would start toggling between the last two history elements(because they are keep getting added to the history).
I see travis isn't happy about me, but I don't really understand from the travis log, which test is going wrong. Could you maybe give some more information on how to resolve this? When I run "scripts\test" on windows everything seems to be passing.

[sandy081]

@wimspaargaren Thats cool and this is what I mentioned in my comment.

The test failing in the Travis build is due to the other changes in the stream. Its not because of this so no need to worry.

LGTM. Thanks for the PR. Merged.