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.

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

Register editor feature for generic multi-doc wordHighlighter #196240

Closed
Yoyokrazy opened this issue Oct 23, 2023 · 1 comment · Fixed by #224884
Closed

Register editor feature for generic multi-doc wordHighlighter #196240

Yoyokrazy opened this issue Oct 23, 2023 · 1 comment · Fixed by #224884
Assignees
Labels
debt Code quality issues editor-highlight Editor selection/word highlight issues

Comments

@Yoyokrazy
Copy link
Contributor

Clean up the code within MainThreadLanguageFeatures.ts with a generic registered editor feature. Also allows for deleting the TextualOccurenceRequest classes from WordHighlighter.ts. Should also be somewhat applicable to standard single doc highlighting.

It's a little surprising to find this here. It's OK'ish but somewhat coupled to the extension host. I think it would be leaner to register an editor feature (registerEditorFeature) which simply uses getOccurrencesAtPosition in combination with the logic below

Originally posted by @jrieken in #196024 (comment)

Also relevant: #196024 (comment)

@Yoyokrazy
Copy link
Contributor Author

Update on progress here (copied from PR comment):

  • Having issues with the textual highlight provider registered editor getting called when the typescript highlight provider returns a valid (albeit empty) result. The current check leverages first and returns when a provider gives a result that evaluates true with result instanceof ResourceMap && result.size > 0. Current (not-functional) code:
// in order of score ask the occurrences provider
// until someone response with a good result
// (good = none empty array)
return first<ResourceMap<DocumentHighlight[]> | null | undefined>(orderedByScore.map(provider => () => {
	const filteredModels = otherModels.filter(otherModel => {
		return score(provider.selector, otherModel.uri, otherModel.getLanguageId(), true, undefined, undefined) > 0;
	});

	return Promise.resolve(provider.provideMultiDocumentHighlights(model, position, filteredModels, token))
		.then(undefined, onUnexpectedExternalError);
}), (t: ResourceMap<DocumentHighlight[]> | null | undefined): t is ResourceMap<DocumentHighlight[]> => t instanceof ResourceMap && t.size > 0);
  • My thought was to shift to a "good" result being = non-null and non-undefined. In other words, a Resource map could be returned containing any number of given URIs and simply not have any DocumentHighlights, and that would evaluate to true and pop out of first(). That changed the final line of the above snippet to:
(t: ResourceMap<DocumentHighlight[]> | null | undefined): t is ResourceMap<DocumentHighlight[]> => t instanceof ResourceMap && t !== null && t !== undefined);
  • That continued to not pop out of first() so my further thought is that likely the typescript highligh provider was returning an empty MultiDocHighlight array upon no results. I then tweaked the implementation within documentHighlight.ts in typescript-lang-feat and set it to return undefined upon a body length of 0 (body of the response is just an empty array when clicking import). This still doesn't pop out of first(), leaving me a bit stumped on where to take this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues editor-highlight Editor selection/word highlight issues
Projects
None yet
2 participants