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

bugfix: call lspCheckForChanges in more places, such as after a failed merge #5545

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

mitchellwrosen
Copy link
Member

@mitchellwrosen mitchellwrosen commented Jan 16, 2025

Overview

Fixes LSP for in-progress merges. I couldn't find a ticket for this issue, but basically, LSP wouldn't properly refresh on merge failure.

Now, it does. I've tested this change manually by setting up the following merge:

LCA

foo = 17
bar = foo + foo

Alice

foo = 18 -- conflicts with bob

Bob

foo = 19 -- conflicts with alice
bar = foo + baz
baz = 200

When merging bob into alice, we end up with a file that looks like

-- alice
foo = 18
-- bob
foo = 19

-- dependents
bar = foo + baz

where baz is in the underlying namespace, because it was an unconflicted add.

Prior to this PR, LSP would complain that baz is out of scope.

call it on `cd`/`switch`/`popd` as well as when the contents of the current branch change
@mitchellwrosen mitchellwrosen marked this pull request as ready for review January 16, 2025 17:08
@mitchellwrosen
Copy link
Member Author

The CI failures are spurious

@aryairani aryairani merged commit 7e7836a into trunk Jan 16, 2025
28 of 32 checks passed
@aryairani aryairani deleted the 25-01-15-lsp-merge-bug branch January 16, 2025 17:29
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