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

find in files - inconsistency in marking results as "dirty" #95779

Closed
JacksonKearl opened this issue Apr 21, 2020 · 2 comments
Closed

find in files - inconsistency in marking results as "dirty" #95779

JacksonKearl opened this issue Apr 21, 2020 · 2 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues verified Verification succeeded
Milestone

Comments

@JacksonKearl
Copy link
Contributor

BTW looks like there is inconsistency in marking results as "dirty" - removing file from result prevents next search to be performed immediately (requires Enter ), but removing one match from file that has several - allows immediate search.

Originally posted by @IllusionMH in #91901 (comment)

@JacksonKearl JacksonKearl self-assigned this Apr 21, 2020
@JacksonKearl JacksonKearl added the search Search widget and operation issues label Apr 21, 2020
@JacksonKearl JacksonKearl added this to the April 2020 milestone Apr 21, 2020
@JacksonKearl JacksonKearl added the bug Issue identified by VS Code Team member as probable bug label Apr 21, 2020
@IllusionMH
Copy link
Contributor

Looks like middle ground and place where logic is missing is

private onFileChange(fileMatch: FileMatch): void {
let added: boolean = false;
let removed: boolean = false;
if (!this._fileMatches.has(fileMatch.resource)) {
this.doAdd(fileMatch);
added = true;
}
if (fileMatch.count() === 0) {
this.doRemove(fileMatch, false, false);
added = false;
removed = true;
}
if (!this._replacingAll) {
this._onChange.fire({ elements: [fileMatch], added: added, removed: removed });
}
}

This one always receives FileMatch and can only calculate if whole file was added/removed, but doesn't know if it was "updated" therefore won't be handled later

if (e.removed) {
this._hasRemovedResults = true;
}

It feels like onFileChange should also receive optional parameter - changed Match and verify if this match is in its list of removed lines. Is it proper fix or wrong approach?

@JacksonKearl
Copy link
Contributor Author

Ended up making it so the file match will explicitly include whether results are removed in its onchange event.

@roblourens roblourens added the verified Verification succeeded label Apr 30, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jun 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants