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] Config to disable populating cache from display requests #11224

Merged
merged 3 commits into from
May 15, 2023

Conversation

kLabz
Copy link
Contributor

@kLabz kLabz commented May 15, 2023

Since 7321f18, successful "full typing" display requests (which includes diagnostics, find references/implementations) can populate compilation server cache.

This can lead to some issues while this feature is not 100% working yet, like #11177 or #11184 (which would have a proper fix with #11220). Some projects might come across different issues or even do things differently in #if display (that would not impact diagnostics but would impact find references/implementations I think?) that could result in types that would not be suited for final generation.

This PR adds a compilation server configuration to opt out of this, so default behavior is supposed to be left unchanged. I also added tests for above issues, commented out for now with default config (failing until #11220 is merged) and only running with custom config.

This configuration option can be useful as a workaround for such issues, for personal preferences (I'd personally take slow cache over corrupted cache) or for troubleshooting the source of an issue.

I will open a couple PRs on vshaxe / Haxe LSP to allow the configuration to be edited, if this is accepted.

@Simn
Copy link
Member

Simn commented May 15, 2023

As previously stated, this is not a solution. We need a mechanism to partially rebuild the cache, and doing it via diagnostics is a first step towards that. Any problems that arise from that have to be investigated and fixed, not hidden from view.

I might be willing to accept this for the 4.3 branch in order to dodge regressions, but I don't want to have it as a permanent option.

@kLabz kLabz merged commit 6a554bf into development May 15, 2023
kLabz added a commit to kLabz/haxe-languageserver that referenced this pull request May 16, 2023
kLabz added a commit to kLabz/haxe that referenced this pull request Jul 10, 2023
…axeFoundation#11224)

* Add compilation server config to disable populating cache from display requests

* [tests] Add test for 11177 (including a disabled one for now)

* [tests] Add test for 11184 (including a disabled one for now)
@kLabz kLabz deleted the diagnostics-config-to-disallow-caching branch July 10, 2023 09:32
@kLabz kLabz added this to the 4.3 Hotfix candidates milestone Sep 5, 2023
kLabz added a commit that referenced this pull request Nov 15, 2023
…11224)

* Add compilation server config to disable populating cache from display requests

* [tests] Add test for 11177 (including a disabled one for now)

* [tests] Add test for 11184 (including a disabled one for now)
kLabz added a commit that referenced this pull request Nov 15, 2023
…11224)

* Add compilation server config to disable populating cache from display requests

* [tests] Add test for 11177 (including a disabled one for now)

* [tests] Add test for 11184 (including a disabled one for now)
@kLabz kLabz removed this from the 4.3 Hotfix candidates milestone Nov 15, 2023
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