Skip to content

Conversation

toinehartman
Copy link
Member

@toinehartman toinehartman commented Oct 1, 2025

This PR keeps track of files that are open with language Parametric Rascal LSP, for which there is no language registered yet. Once the user registers a language, we update the associated open editors.

This PR closes #795.

Keep track of files that are classified as 'Parametric Rascal MPL'
language, but with an unknown extension. Once someone registers this
extension, store the file state and update the editor as if they were
opened *after* registering the language.
@toinehartman toinehartman self-assigned this Oct 1, 2025
@toinehartman toinehartman added the bug Something isn't working label Oct 1, 2025
@toinehartman toinehartman changed the title Remember files opened as 'Parametric Rascal MPL before language registration Remember files opened as Parametric Rascal MPL before language registration Oct 1, 2025
@toinehartman toinehartman changed the title Remember files opened as Parametric Rascal MPL before language registration Remember files opened as Parametric Rascal LSP before language registration Oct 1, 2025
Copy link
Member

@DavyLandman DavyLandman left a comment

Choose a reason for hiding this comment

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

I think this PR is a bit involved, as in it requires many places to take into account possible race between registry & availability of the language. Especially if someone later adds a new capability, it'll soon be forgotten.

I'm thinking there might be a way to make a subclass of our ParsingOnlyContribution (like EmptyContribution) and then connect that to the incoming file. and when a registry comes in, we update all the existing mappings to the new contribution?

@toinehartman toinehartman force-pushed the fix/795-open-register-race branch from 5c9467d to 1a0da53 Compare October 6, 2025 14:16
@toinehartman toinehartman marked this pull request as ready for review October 6, 2025 14:43
Copy link
Member

@DavyLandman DavyLandman left a comment

Choose a reason for hiding this comment

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

I like how small this got in the end.

Copy link
Member

@DavyLandman DavyLandman left a comment

Choose a reason for hiding this comment

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

This doesn't feel complete to me, or else I'm missing some connections.

Copy link
Member

@DavyLandman DavyLandman left a comment

Choose a reason for hiding this comment

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

Yes, this is much better, I have some small questions and stuff we need to improve.

Copy link

Copy link
Member

@DavyLandman DavyLandman left a comment

Choose a reason for hiding this comment

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

Two small questions, for the rest it looks find

);
var loc = Locations.toLoc(doc);
return files.computeIfAbsent(loc,
l -> new TextDocumentState(contributions(loc)::parsing, l, doc.getVersion(), doc.getText(), timestamp));
Copy link
Member

Choose a reason for hiding this comment

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

l and loc are the same, no need to capture them from outside the closure.

(I don't see why we need this change?)

diagnosticsAsync.complete(diagnostics);
});
} catch (NoContributionException e) {
logger.debug("Ignoring missing parser for {}", location, e);
Copy link
Member

Choose a reason for hiding this comment

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

Is it no problem that treeAsync is never completed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

If a DSL file is already open when it's registered, the user gets a whole lot of error messages

3 participants