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

[display] Remove legacy diagnostics #11413

Draft
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

kLabz
Copy link
Contributor

@kLabz kLabz commented Nov 25, 2023

⚠️ Merge later

Should be merged sometime next year I suppose?

Closes #8086

@kLabz kLabz added the feature-ide IDE / Editor support label Nov 25, 2023
@kLabz kLabz added this to the Release 5.0 milestone Nov 25, 2023
@skial skial mentioned this pull request Nov 28, 2023
1 task
@Simn
Copy link
Member

Simn commented Aug 2, 2024

Should we do this now?

@kLabz
Copy link
Contributor Author

kLabz commented Aug 2, 2024

Well, hmm

  • Latest vshaxe release still doesn't support Json RPC diagnostics (merged but not released)
  • The number of people that have actually used Json RPC diagnostics is likely < 5 atm; we might want to release Haxe 5 preview with both, and then merge this

... which actually tells me there's potentially something missing before doing a vshaxe release: there's currently no way to opt out of Json RPC diagnostics, and in short term that might be annoying if it ends up being "broken" 🤔

cc @AlexHaxe

@AlexHaxe
Copy link
Contributor

AlexHaxe commented Aug 2, 2024

I think we should put a useLegacyDiagnostics config option in vshaxe. and it probably should default to false so that people use new diagnostics in 4.3.5+ and 5, because nobody's gonna turn it on unless it's heavily advertised.

and then we should release a new vshaxe version soonish. I have two fixes for tokentree that should go in.
there are a couple of issues with rename lib, but I haven't had the time to fix those. they can go into a later release when they are ready so it shouldn't be blocking a Json RPC enabled vshaxe release.

@kLabz
Copy link
Contributor Author

kLabz commented Aug 2, 2024

Note that such a config option wouldn't work with Haxe 5 (and, more importantly, would make diagnostics fail if vshaxe tries to honor it on a Haxe version where this PR is merged)

@AlexHaxe
Copy link
Contributor

AlexHaxe commented Aug 2, 2024

true. we could use context.haxeServer.haxeVersion to limit it to supported versions (e.g. < 5.0.0) once this PR is merged. obviously that would leave nightlies without Json RPC with no diagnostics. but at least the stable release people will have an opt-out (not sure if there are people relying on nightlies for their projects).

AlexHaxe added a commit to AlexHaxe/haxe-languageserver that referenced this pull request Aug 4, 2024
AlexHaxe added a commit to vshaxe/haxe-language-server that referenced this pull request Aug 4, 2024
… 4.3.5+ (#129)

* added useLegacyDiagnostics config for opting out of Json RPC for Haxe 4.3.5+
see HaxeFoundation/haxe#11413
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-ide IDE / Editor support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Diagnostics error occurs when an @ is used in the file path
3 participants