Origin/lsp #23
Origin/lsp #23
Conversation
src/extension.ts
Outdated
| // The L2 version, if less than v1.5.0, does not support the `l2 -e <filepath>` feature. | ||
| // Therefore, we use the extension to fetch variables from the l2.env file for suggestions. | ||
| /** | ||
| * The L2 version, if less than v1.5.0, does not support the `l2 -e <filepath>` feature. |
There was a problem hiding this comment.
Let's ditch this feature. We'll go 100% LSP from v1.5.9. We can remove it from backend in v1.5.9 as well.
src/extension.ts
Outdated
| // Therefore, we fetch variables from both the l2.env and l2config.env files for suggestions using this command. | ||
| suggestEnvVariables = lama2RegisterCompletionItemProvider(); | ||
| /** | ||
| * L2 versions greater than or equal to v1.5.0 support the `l2 -e <filepath>` feature. |
There was a problem hiding this comment.
No need to retain this as well. Only LSP going forward.
src/lsp/methods/lspLifecycles.ts
Outdated
| sendRequestToLSPReadResponse, | ||
| } from "../request/generalRequest"; | ||
|
|
||
| export function initlizeServer(langServer: ChildProcess, requestId: number) { |
| ErrorCodes, | ||
| IJSONRPCResponse, | ||
| logToChannel, | ||
| } from "../response/generalResponse"; |
There was a problem hiding this comment.
We can get imports based on project root everywhere if possible.
There was a problem hiding this comment.
Will do in the next MR, tried, taking a lot of time
|
|
||
| return completions; | ||
| if (isCursorInsideTemplateLiteral(lineText, cursorPosition.character)) { |
src/lsp/request/generalRequest.ts
Outdated
| } | ||
|
|
||
| // Function to send requests to the Language Server Protocol and read responses | ||
| export function sendRequestToLSPReadResponse( |
There was a problem hiding this comment.
Simplify name - maybe something like: askLSP (and response type is clear by return type)
src/lsp/request/generalRequest.ts
Outdated
|
|
||
| const chunks: any[] = []; | ||
| process.stdout.on("data", (data) => { | ||
| chunks.push(data); |
There was a problem hiding this comment.
You can put these event handlers as separate as functions; as of now this function is a bit too long and not super easy to read.
There was a problem hiding this comment.
Right addressed it
| const { msg: message, logType = "info", dataObject, dataString } = options; | ||
|
|
||
| const timestamp = new Date().toLocaleTimeString(); | ||
| const logTypeInfo = LOG_TYPES[logType.toLowerCase()] || LOG_TYPES["info"]; |
There was a problem hiding this comment.
Isn't L45 setting logType default value? I don't think we need ||
There was a problem hiding this comment.
Acts like default value
logToChannel({
msg: "Received envs from server",
dataObject: response,
});
What type of MR is this? (check all applicable)
Description
l2 -e='your_search_query'using the LSP built for l2.Important files to start review from
Mobile & Desktop Screenshots/Recordings
Bug fix for envs within string enclosed env placeholder
Unsupported error for windows
Added tests?
Added to documentation?
Prettify the repositary
npm run format