Skip to content

[NETBEANS-5326] LSP codelens output fixes#2725

Merged
sdedic merged 3 commits intoapache:deliveryfrom
entlicher:vscodeext_CodelensOutputFixes
Feb 11, 2021
Merged

[NETBEANS-5326] LSP codelens output fixes#2725
sdedic merged 3 commits intoapache:deliveryfrom
entlicher:vscodeext_CodelensOutputFixes

Conversation

@entlicher
Copy link
Contributor

@entlicher entlicher commented Feb 3, 2021

This is a fix of NETBEANS-5326

If Run test/Debug test code lenses are used through LSP protocol, the actual test output is not visible. In case of Debug tes, the output is visible when it succeeds, however, it is significantly cut when it fails, preventing from seeing the test failure.

Correction of output from Run/Debug code lens actions.

  • In case of Debug, the TerminatedEvent was sent too soon. Now, similar to Run, the TerminatedEvent is sent when the application terminates.
  • Run now runs in the same was as "Run Without Debug" - through startDebugger with noDebug = true. This is necessary for consistency and proper output display.
  • A NPE associated with a missing source path in case of Run is addressed

@entlicher entlicher added kind:bug Bug report or fix LSP [ci] enable Language Server Protocol tests labels Feb 3, 2021
@entlicher entlicher added this to the 12.3 milestone Feb 3, 2021
@entlicher entlicher changed the title Vscodeext codelens output fixes LSP codelens output fixes Feb 3, 2021
Copy link

@JaroslavTulach JaroslavTulach left a comment

Choose a reason for hiding this comment

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

Sharing the Run and Debug and single and all execution is certainly good idea.

@neilcsmith-net
Copy link
Member

At the moment, we don't have another beta scheduled, although discussions elsewhere might lead to one. If not, I don't feel in a position to assess this one against the bug priority guidelines. Can we get assessment of the necessity to get this in to 12.3? Thanks!

@entlicher
Copy link
Contributor Author

@neilcsmith-net I've updated the description and created NETBEANS-5326 based on our internal report. That should allow to asses the importance for 12.3. I consider the fix to be of a very low risk, especially comparing to the currently broken behavior.
The modifications are in the area of the LSP server implementation only.

@neilcsmith-net neilcsmith-net changed the title LSP codelens output fixes [NETBEANS-5326] LSP codelens output fixes Feb 4, 2021
@neilcsmith-net
Copy link
Member

@entlicher thanks. It's more about criticals and blockers generally requiring another beta for testing. Quite happy for major (or minors) to come in then where available, particularly when they're low risk! @geertjanw and I put forward an optimistic schedule with only two betas initially due to late start on the release (no RM in place). At least one more beta is looking likely now anyway.

I've updated the title - hopefully Apache infra will link the two.

CompletableFuture.runAsync(() -> {
try {
waitForDebuggeeConsole.get(5, TimeUnit.SECONDS);
waitForDebuggeeConsole.get(1, TimeUnit.SECONDS);
Copy link
Member

Choose a reason for hiding this comment

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

Hi, incidentally I attempted to solve this delay by removing it :) completely in #2754 . See the discussion in that PR.

@sdedic
Copy link
Member

sdedic commented Feb 11, 2021

Merged per Neil's advice in #2754

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

Labels

kind:bug Bug report or fix LSP [ci] enable Language Server Protocol tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants