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

Let LSP client decide on source name #103

Merged
merged 1 commit into from
May 6, 2023

Conversation

kLabz
Copy link
Contributor

@kLabz kLabz commented May 6, 2023

No description provided.

@Simn Simn merged commit 28c1cae into vshaxe:master May 6, 2023
2 checks passed
@@ -183,8 +180,7 @@ class DiagnosticsFeature {
};
final diag:Diagnostic = {
range: range,
source: DiagnosticsSource,
code: kind,
code: hxDiag.code,
Copy link
Member

Choose a reason for hiding this comment

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

my tests with nightly and 4.3.x always had hxDiag.code as null and since kind isn't preserved (or used as code) our diagnostic actions fail to contribute any quickfixes.

so how is the new code system supposed to work? and what do we need to adjust to fix our code actions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need HaxeFoundation/haxe#11207 to get the code from Haxe (which wouldn't give the values you expect)
Using code to decide what quickfix to add sounds hacky, can't/shouldn't tags be used?

Copy link
Member

Choose a reason for hiding this comment

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

currently DiagnosticTag has two values: Unnecessary and Deprecated. technically it's an Int and we could cast any number to it, and it might work.
we probably should take some higher values for our tags though, to avoid potential conflict with future extensions of that enum on vscode's side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should take a look at what we actually used that code (well, kind) for and see how I could "un-break" it without breaking the rest of the features I was working on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there more than DiagnosticsCodeActionFeature to handle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I see several more using context.diagnostics.getArguments() which may still be working, but I'll have to check those too.

Copy link
Member

@AlexHaxe AlexHaxe May 7, 2023

Choose a reason for hiding this comment

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

not that I'm aware of you are right, at least one uses code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems to work fine with this PR: #107

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants