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

Use Number.MAX_SAFE_INTEGER in file quick search #11232

Merged

Conversation

colin-grant-work
Copy link
Contributor

What it does

Despite #10694, it is still possible to get good matches ranked ahead of perfect matches, because the MAX_SAFE_INTEGER constant used was not necessarily the actual maximum safe integer available in the browser, and fuzzy search could still return larger numbers.

How to test

  1. Tests should pass.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added the file search issues related to the file search label May 31, 2022
@colin-grant-work
Copy link
Contributor Author

@msujew, should we just use Number.MAX_SAFE_INTEGER everywhere, you think?

@msujew msujew self-requested a review May 31, 2022 22:30
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

@colin-grant-work Yes and no. There are two kinds of MAX_SAFE_INTEGER cases:

  1. We actually want the larges possible integer available in the runtime (2^53 - 1)
  2. We want the largest possible integer that monaco accepts (2^32 - 1)

So maybe rename the exported MAX_SAFE_INTEGER to MAX_SAFE_MONACO_INTEGER? As far as I can tell, the change that you performed is the only instance where we use the exported MAX_SAFE_INTEGER variable unintentionally.

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@colin-grant-work colin-grant-work merged commit 0730d3b into eclipse-theia:master Jun 7, 2022
@colin-grant-work colin-grant-work deleted the bugfix/sorting-again branch June 7, 2022 14:03
@colin-grant-work colin-grant-work added this to the 1.27.0 milestone Jun 7, 2022
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants