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

Diagnostics delayed by 1 keystroke #26

Closed
rzgry opened this issue Oct 27, 2020 · 8 comments
Closed

Diagnostics delayed by 1 keystroke #26

rzgry opened this issue Oct 27, 2020 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@rzgry
Copy link
Member

rzgry commented Oct 27, 2020

Diagnostics added by #23 are always 1 keystroke behind.

@kathrynkodama
Copy link
Contributor

@PENGYUXIONG
Copy link
Contributor

I dont have much progress on this bug still.

My thought is that, either
https://github.com/MicroShed/jakarta-ls/blob/26641efb51a3fbd0e2f05118d0d4f1548395418d/jakarta-eclipse/src/org/jakarta/jdt/JDTUtils.java#L117

is returning incorrect Ifile which I am not sure how to verify

or it can possibly be JavaCore.createCompilationUnitFrom(resource); from
https://github.com/MicroShed/jakarta-ls/blob/26641efb51a3fbd0e2f05118d0d4f1548395418d/jakarta-eclipse/src/org/jakarta/jdt/JDTUtils.java#L125 is not behave properly

@PENGYUXIONG PENGYUXIONG removed their assignment Nov 20, 2020
@kathrynkodama
Copy link
Contributor

Adding the following to https://github.com/MicroShed/jakarta-ls/blob/master/jakarta.ls/src/main/java/io/microshed/jakartals/JakartaLanguageServer.java#L41 will update diagnostics on save:

serverCapabilities.setTextDocumentSync(TextDocumentSyncKind.Incremental);

Keystroke delay still exists without saving

@tiganov
Copy link
Contributor

tiganov commented Sep 23, 2021

Adding a delay before createCompilationUnitFrom is called in JDTUtils.java seems to resolve the keystroke delay, but I'm not quite sure why, yet.

try {
    Thread.sleep(1000);
} catch (InterruptedException e) {
    e.printStackTrace();
}
return JavaCore.createCompilationUnitFrom(resource);

@kathrynkodama kathrynkodama added this to New in CANOSP Fall 2021 via automation Sep 24, 2021
@kathrynkodama kathrynkodama moved this from New to In progress in CANOSP Fall 2021 Sep 24, 2021
@tiganov
Copy link
Contributor

tiganov commented Oct 29, 2021

It appears ICompilationUnit.isConsistent returns false immediately after the unit is created in JDTServicesManager.getJavaDiagnostics(). Introducing a sufficient delay (like I've shown in my previous comment) will allow the underlying resource to catch up (and then isConsistent will return true). Therefore, adding a while loop to allow the unit to catch up solves the keystroke delay.

public List<PublishDiagnosticsParams> getJavaDiagnostics(JakartaDiagnosticsParams javaParams) {
    [...]
        ICompilationUnit unit = JDTUtils.resolveCompilationUnit(u);
        try {
            while (!unit.isConsistent()) { }
        } catch (JavaModelException e) { }
        for (DiagnosticsCollector d : diagnosticsCollectors) {
            d.collectDiagnostics(unit, diagnostics);
        }
    [...]
}

Alternatively, the loop can be added to JDTUtils.resolveCompilationUnit, instead.

@kathrynkodama
Copy link
Contributor

It appears ICompilationUnit.isConsistent returns false immediately after the unit is created in JDTServicesManager.getJavaDiagnostics(). Introducing a sufficient delay (like I've shown in my previous comment) will allow the underlying resource to catch up (and then isConsistent will return true). Therefore, adding a while loop to allow the unit to catch up solves the keystroke delay.

public List<PublishDiagnosticsParams> getJavaDiagnostics(JakartaDiagnosticsParams javaParams) {
    [...]
        ICompilationUnit unit = JDTUtils.resolveCompilationUnit(u);
        try {
            while (!unit.isConsistent()) { }
        } catch (JavaModelException e) { }
        for (DiagnosticsCollector d : diagnosticsCollectors) {
            d.collectDiagnostics(unit, diagnostics);
        }
    [...]
}

Alternatively, the loop can be added to JDTUtils.resolveCompilationUnit, instead.

This looks great, thanks @tiganov! Any concerns with unit.isConsistent() never returning true and getting stuck in that while loop?

@tiganov
Copy link
Contributor

tiganov commented Oct 29, 2021

@kathrynkodama I haven't had issues with the while loop getting stuck, but I will put a timeout condition just in case.

tiganov added a commit to tiganov/lsp4jakarta that referenced this issue Oct 29, 2021
kathrynkodama pushed a commit that referenced this issue Oct 29, 2021
* Fixed keystroke delay (#26)

* Changed ICompilationUnit update timeout to a constant value
@kathrynkodama
Copy link
Contributor

Closed with #201

CANOSP Fall 2021 automation moved this from In progress to Done Oct 29, 2021
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
Development

No branches or pull requests

4 participants