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

file-search: fix possible infinite recursion when comparing items #9635

Merged
merged 1 commit into from
Jun 25, 2021

Conversation

vince-fugnitto
Copy link
Member

What it does

The pull-request addresses a possible infinite recursion when comparing two identical quick-open-items (namely equal label and uri). Instead of causing the infinite recursion, we will exit early if the uri comparison is equal (value 0).

The changes also include api-tests for file-search.

How to test

  • confirm that the quick-open functionality still works correctly - ctrlcmd+p
  • confirm that the new api-tests pass successfully

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto [email protected]

@vince-fugnitto vince-fugnitto added quality issues related to code and application quality file search issues related to the file search test issues related to unit and api tests labels Jun 23, 2021
@vince-fugnitto vince-fugnitto self-assigned this Jun 23, 2021
@vince-fugnitto vince-fugnitto force-pushed the vf/file-search-recursion branch 3 times, most recently from e3a3a9c to 766eff1 Compare June 23, 2021 18:46
Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

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

Code LGTM, good catch with the recursion.

The commit fixes the possible infinite recursion when comparing two
identical quick-open-items, namely with equal `label` and `uri`.

The code to compare items compares them first by:
- `label`.
- `uri` if the labels are equal.

The issue is that it attempts to compare `uri` recursively if it is
equal.

Signed-off-by: vince-fugnitto <[email protected]>
@vince-fugnitto vince-fugnitto force-pushed the vf/file-search-recursion branch from 766eff1 to 0804a54 Compare June 23, 2021 18:48
Copy link
Contributor

@alvsan09 alvsan09 left a comment

Choose a reason for hiding this comment

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

Good find!
Good to see the benefits of unit testing ! 👍

@vince-fugnitto vince-fugnitto merged commit 144da47 into master Jun 25, 2021
@vince-fugnitto vince-fugnitto deleted the vf/file-search-recursion branch June 25, 2021 16:37
@github-actions github-actions bot added this to the 1.15.0 milestone Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
file search issues related to the file search quality issues related to code and application quality test issues related to unit and api tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants