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

Fix issue with ordering of module opened via -open #842

Merged
merged 2 commits into from
Nov 11, 2023

Conversation

zth
Copy link
Collaborator

@zth zth commented Nov 11, 2023

This fixes a very annoying issue where you wouldn't get autocomplete for "second order opens". I haven't added any tests because it'd be an incredibly noisy diff, but here's roughly what was wrong and what it caused:

  1. Say you have an open in bsc-flags, like -open RescriptCore. Everything works as expected
  2. The RescriptCore open exposes a bunch of modules like Array, List, Result etc. All of these are available in autocomplete. So far, so good.
  3. Now, let's say you were to add an explicit open in a file for a module exposed via -open RescriptCore, like open Array.
  4. The content of Array won't be exposed to autocomplete in this case.

As can be seen by the minimal diff, this was a case of ordering. The list is reversed (correctly), but the order of rawOpens (coming from the local file) and packageOpens (coming from the package, bsconfig/rescript.json) was off, so the package opens actually ended up after the local opens when resolving autocomplete. This in turn lead to the contents of the opened modules not being exposed.

Phew.

@zth zth requested a review from cristianoc November 11, 2023 18:13
Copy link
Collaborator

@cristianoc cristianoc left a comment

Choose a reason for hiding this comment

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

Seems legit.

@zth zth merged commit 2c2552e into master Nov 11, 2023
5 checks passed
@zth zth deleted the fix-bsconfig-open-order branch November 11, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants