Leverage LSP to do compile instead of CLI #7239
Conversation
chunyu3
requested review from
RodgeFu,
bterlson,
markcowl,
allenjzhang,
timotheeguerin,
chrisradek,
AlitzelMendez,
lirenhe and
witemple-msft
as code owners
|
You can try these changes here
|
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
| finalMessage: string, | ||
| asyncAction: (task: TrackActionTask) => Promise<T>, | ||
| ): Promise<T> { | ||
| const task = new DynamicServerTask(message, finalMessage, serverHost.log); |
There was a problem hiding this comment.
is this now logging all overt the place? This compile service is called a lot in the LSP
There was a problem hiding this comment.
This is only logged when emit code. Other compiles such as compiling for authoring does not.
There was a problem hiding this comment.
I guess its more can't this be defined in the server host construction instead of injected here for some irrelevant behavior of the compile service?
There was a problem hiding this comment.
There will be only one ServerHost instance used across all scenarios, including authoring, hovering, and others. In these general scenarios, we do not log compile-time tracking information. Therefore, the trackAction will be undefined by default.
However, when handling the emit-code operation, we do need to log tracking data. In that case, we explicitly set trackActionand wrap the ServerHost in a new instance to ensure this logging does not interfere with other scenarios like authoring.
| @@ -112,17 +119,22 @@ export function createCompileService({ | |||
| */ | |||
| async function compile( | |||
| document: TextDocument | TextDocumentIdentifier, | |||
| additionalOptions?: CompilerOptions, | |||
| bypassCache: boolean = false, | |||
There was a problem hiding this comment.
whats the purposse of bypassCache?
thoses options should also be an option bag not boolean positional
There was a problem hiding this comment.
When there is a cache, and it is the same as before, compiling will complete after parse the tsp files and emitting will be cancelled.
So for emit-code request, we will bypassCache
| @@ -53,3 +53,8 @@ export async function TraverseMainTspFileInWorkspace() { | |||
| .map((uri) => normalizeSlashes(uri.fsPath)), | |||
| ); | |||
| } | |||
|
|
|||
| export function GetVscodeUriFromPath(path: string) { | |||
There was a problem hiding this comment.
camelCase
isn't that what the fileService already gives you too?
There was a problem hiding this comment.
VscodeUri is encoded. fileService provide non-encoded one
e.g.
fileService: c:\dev\demoProject\main.tsp
vscodeuri: file:///c%3A/dev/demoProject/main.tsp
in lsp server cache, it will use vscodeUri as the key,
Co-authored-by: Timothee Guerin <timothee.guerin@outlook.com>
Fix #6663
Now we are having many problems when triggering the compile from CLI and handling its output in our Output window like multiple loglevel, how to handle ```stderr``, coloring and so on.
So to resolve all these problems, let's trigger compile through LSP instead of CLI so that we can retrieve the result of compile directly which is more reliable and also easier to do post-processing.