Initial bits for integrating the LSP API.#609
Draft
christianparpart wants to merge 9 commits intomasterfrom
Draft
Initial bits for integrating the LSP API.#609christianparpart wants to merge 9 commits intomasterfrom
christianparpart wants to merge 9 commits intomasterfrom
Conversation
christianparpart
force-pushed
the
lsp-integration
branch
from
9e4af31 to
e6942a8
Compare
christianparpart
force-pushed
the
lsp-integration
branch
5 times, most recently
from
1c76e96 to
d66dd93
Compare
chriseth
reviewed
Feb 22, 2022
Contributor
|
Looks good in general! Maybe we can actually just ignore the synchronous read problem for the first version. Also @axic had some additional ideas for unifying the callback functions. |
stephen-slm
reviewed
Mar 3, 2022
stephen-slm
reviewed
Mar 3, 2022
stephen-slm
reviewed
Mar 3, 2022
stephen-slm
reviewed
Mar 3, 2022
stephen-slm
reviewed
Mar 3, 2022
stephen-slm
reviewed
Mar 3, 2022
stephen-slm
suggested changes
Mar 3, 2022
Contributor
There was a problem hiding this comment.
Legacy Coding Style
All changes show entirely legacy coding style for Javascript and goes against best practices. Additionally the code is made increasingly more complicated than required with multiple nested functions which should not be const functions but well defined for typing.
- Interfaces should be moved outside of wrapper.ts file.
- no supporting tests for changes.
- no linting
Just my two cents.
christianparpart
force-pushed
the
lsp-integration
branch
from
a21be10 to
85a853e
Compare
stephen-slm
reviewed
Mar 14, 2022
| @@ -79,6 +79,8 @@ function setupMethods (soljson) { | |||
| const copyFromCString = soljson.UTF8ToString || soljson.Pointer_stringify; | |||
|
|
|||
| const wrappedReadCallback = function (path: string, contents: string, error: string) { | |||
Contributor
There was a problem hiding this comment.
just run npm run lint:fix or the related package.json command to resolve these before committing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.