Add history navigation for file include/exclude patterns in the search pane. Remember history between sessions.

[stringham]

This implements #20547 for both the include and exclude patterns.

It also remembers the history for the search terms and the include/exclude patterns on shutdown and startup.

[mention-bot]

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

[msftclas]

@stringham,
Thanks for having already signed the Contribution License Agreement. Your agreement has not been validated yet. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

[sandy081]

@stringham Changes look good to me. Can you please resolve the conflicts so that I will make a merge?

Thanks

[stringham]

It appears that this commit: bdc326b broke history.ts, because the native Set doesn't have order built in and the ._limit functionality isn't removing the least recently used item.

[sandy081]

@stringham Native Set iterator gives the elements in the insertion order - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set/forEach

So, I do not think, adopting to native Sethas not changed any semantics. Please let me know if you see any change in behaviour.

Thanks.

[stringham]

Okay, I saw a couple of weird things. 1 of them was that I was adding the empty string to the history and showPrevious and showNext make sure you get a truthy value back from the history.

I fixed that by checking before adding to the history (should the empty string be allowed in the history?)

this._register(this.onSubmit(() => {
	let value = this.getValue();
	if(value.length > 0) {
		this.history.add(this.getValue());
	}
}));

the issue with history can be seen with this unit test:

test('adding existing should move it to the end', function() {
	let testObject = new HistoryNavigator([], 5);
	testObject.add('1');
	testObject.add('2');
	assert.equal('2', testObject.current());
	testObject.add('1');
	assert.equal('1', testObject.current());
});

and can be fixed by doing this in history.ts:

public add(t: T) {
	this._history.add(t);
	this._onChange();
}

ArraySet was previously handling this:

set(element: T): void {
	this.unset(element);
	this._elements.push(element);
}

[stringham]

I've resolved merge conflicts for this feature and it should be ready to merge when you're ready.

[stringham]

@sandy081 I believe this is good to go. Do you want me to open a pull request to fix history.ts?

[sandy081]

@stringham Thanks for the changes.. We froze changes for this milestone, this Monday and I will merge this PR when the master is open which is mostly next Monday.

Regarding issue with history, please open a separate issue. We can discuss there. I like to have one fix per issue.

Thanks.

[stringham]

@sandy081 is this waiting on anything else?

[sandy081]

@stringham No.. it is waiting for my time :). Sorry, Its in my list and will merge this soon.

Thanks.

[brunolemos]

Does this or any other PR adds history to the "Replace" feature as well?

[sandy081]

@brunolemos Not yet. Feel free to contribute.

[efc]

Having the search history that is present right now is very helpful, thank you to all who work on these features.

However, I think it is a bit awkward still. It appears that each "supported" field (the search field and the file include and file exclude fields) seems to have an independent history. But that is not how I, at least, use search.

When I search, I work a while to construct a proper search, which includes decisions about the search term, the modifications of the search (case sensitivity and such), the appropriate replacements, and the files to include/exclude. It is really a package deal, all of it together is what represents a search, not any one field alone.

Right now I have to arrow in at least three fields to sync up a package search that was all in sync at the time I last used it. And even then, the replace field and the search attributes (case, grep, etc) are left out and not recalled from history.

So if anyone is still working on this, it would be really great to create a search history that remembers the whole package, everything that went into a given search, and pulls it all back at once. I think the logic of saving the history only once it is navigated is fine, though I would also see a benefit in an explicit choice to "save search" from a new mini-icon at the top of the search panel that could also pop down a list of past saved searches. My model for this might be how BBEdit on the Mac saves searches, with both a history and a more intentional named list of searches.

I apologize for not diving in and building this. I know not the first thing about Electron apps or how VS Code fits together. Maybe someday! But I just wanted to share the idea. Maybe someone who knows more will be inspired.

[roblourens]

That request would be covered by #16488