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

Implement new find functionality in Explorer view #232964

Merged
merged 7 commits into from
Nov 7, 2024
Merged

Conversation

benibenj
Copy link
Contributor

@benibenj benibenj commented Nov 4, 2024

Introduce methods for marking and unmarking items as find results, along with a mechanism to prevent refreshing the view when phantom elements are present. Additionally, add a context key to indicate when the find provider is active.

fixes #233185

@benibenj benibenj self-assigned this Nov 4, 2024
@benibenj benibenj marked this pull request as ready for review November 7, 2024 11:39
@benibenj benibenj enabled auto-merge (squash) November 7, 2024 11:39
@vs-code-engineering vs-code-engineering bot added this to the November 2024 milestone Nov 7, 2024
Copy link
Member

@joaomoreno joaomoreno left a comment

Choose a reason for hiding this comment

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

Some tests would also be nice 😄

try {
node = this.getDataNode(element);
} catch {
console.log('Error in _updateChildren');
Copy link
Member

Choose a reason for hiding this comment

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

This is a bad idea. If this happens, the tree will be in a very bad state and there's no recovering from here. So we want the error to be thrown instead of quietly logging and pretending everything is fine.

Copy link
Member

Choose a reason for hiding this comment

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

If this indeed is something that is happening, we must dig into the root cause and fix the issue upstream.

Copy link
Contributor Author

@benibenj benibenj Nov 7, 2024

Choose a reason for hiding this comment

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

Thanks for catching this, I added it for debugging reasons but forgot to remove it.

@benibenj benibenj requested a review from joaomoreno November 7, 2024 19:51
@benibenj benibenj dismissed joaomoreno’s stale review November 7, 2024 19:52

request has been adressed

@benibenj benibenj merged commit 6ee792e into main Nov 7, 2024
8 checks passed
@benibenj benibenj deleted the benibenj/content-yak branch November 7, 2024 22:41
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Dec 22, 2024
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.

Find in Outline View not working
3 participants