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.

Sign up for GitHub

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

Jump to bottom

refactor: DAP via LSP messages #109

Merged
merged 29 commits into from
Mar 26, 2021
Merged

refactor: DAP via LSP messages #109

merged 29 commits into from
Mar 26, 2021

Conversation

slavek-kucera
Copy link
Contributor

No description provided.

@slavek-kucera slavek-kucera marked this pull request as ready for review March 4, 2021 09:19
@slavek-kucera
send dap message via lsp channel
a44cde2 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
refactor LSP communication details
c0adf86 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
json queue
4a76f83 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
json message router
6861c54 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
client changes to support new server parameters
9f5ee74 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
...
24da509 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
do not push when terminated
66710f1 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
incorrent guards
5aa8d37 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
tunnel DAP through the LSP channel
ca67677 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
add licenses
acc14f1 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
refactor DAP message (un)wrapper
feba302 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
newlines...
9e481bb 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
remove a few warnings
dced1b1 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
...
a160f63 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
clang format
74ae9c8 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
theia compatibility workaround
3f34dcf 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
Add tests
d9b5587 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
refactor dap session handling
c4ad790 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
Separate debugger from workspace_manager
627172d 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
Missing license header
286d8cd 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
Tests & fixes
43ac63d 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
Fixes, code smells ...
7331759 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
Process ALL messages in the buffer
c8287f0 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
...
86a4f23 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
Debugger pimpl
76076a2 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
rename variables
2ca7771 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
Fixes...
14a0eb0 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
Code smells, debugger tests
1de80be 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
michalbali256
michalbali256 reviewed Mar 25, 2021
View reviewed changes
Copy link
Contributor

@michalbali256 michalbali256 left a comment

Choose a reason for hiding this comment

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

Nice PR.

Do you think you could add one more test that would simulate running macro tracer while receiving LSP requests? This is the first PR that allows simultaneous processing of DAP and LSP requests and the test could detect some hard-to-discover bugs in combination with TSAN in the future.

language_server/src/dap/dap_message_wrappers.h Outdated Show resolved Hide resolved
language_server/src/dispatcher.h Show resolved Hide resolved
language_server/src/lsp/channel.cpp Outdated Show resolved Hide resolved
language_server/src/main.cpp Show resolved Hide resolved
language_server/src/main.cpp Outdated Show resolved Hide resolved
language_server/src/main.cpp
ret = lsp_dispatcher.run_server_loop();
auto msg = channel.read();
if (!msg.has_value())
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I understand, the language server ends when the input pipe is destroyed. That is somewhat incompliant with the LSP spec which says that the server should exit after exit notification.

When we didn't have the exit mechanizm implemented correctly in the past, we had problems on Mac OS, where the language_server executable would never end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure it was due to the pipe not being closed rather then the server getting stuck somewhere else?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I am not sure, I just think we should try it somewhere. But I do not have Mac available..

language_server/test/blocking_queue_test.cpp Outdated Show resolved Hide resolved
parser_library/include/debugger.h Show resolved Hide resolved
parser_library/include/sequence.h Outdated Show resolved Hide resolved
parser_library/src/debugging/debugger.cpp Outdated Show resolved Hide resolved
@slavek-kucera
PR suggestions
3a72e63 
Signed-off-by: Slavomir Kucera <slavomir.kucera@broadcom.com>
@slavek-kucera
Copy link
Contributor Author

Regarding the extra test - I think there is an existing problem where debug_lib_provider holds on to a reference to an object that could be dead. This PR certainly won't help with that, so I am thinking of moving to shared_ptr in the future.

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 37 Code Smells

79.1% 79.1% Coverage
0.0% 0.0% Duplication

@michalbali256 michalbali256 merged commit 044acad into eclipse-che4z:development Mar 26, 2021
@slavek-kucera slavek-kucera deleted the dap-via-lsp branch April 26, 2021 13:57
@github-actions
Copy link

🎉 This PR is included in version 0.13.0-beta.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@github-actions
Copy link

github-actions bot commented Jun 1, 2021

🎉 This PR is included in version 0.13.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

@michalbali256 michalbali256 michalbali256 left review comments

Assignees
No one assigned
Labels
released on @release-0.13.0 released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@slavek-kucera @michalbali256