-
Notifications
You must be signed in to change notification settings - Fork 44
overview documentation of the extension #857
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.
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
Conversation
| createPreviewPanel: ( | ||
| viewType: string, | ||
| title: string, | ||
| preserveFocus?: boolean, | ||
| options?: WebviewPanelOptions & WebviewOptions | ||
| ): WebviewPanel => { | ||
| return window.createWebviewPanel(viewType, title, { viewColumn: ViewColumn.Beside, preserveFocus, }, options); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was previously imported from preview.ts where it was defined by itself. I found that hard to navigate and also the implementation of this function in the other ExtensionHost in hooks.ts defines this inline.
| private async hasExecutorForLanguage(language: string, document: TextDocument, engine: MarkdownEngine) { | ||
| // TODO: this is incorrect right? `cellExecutorForLanguage` returns a promise, and a promise will always be truthy? | ||
| // We should have to await it before doing `!!` | ||
| return !!this.cellExecutorForLanguage(language, document, engine); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't want to make a change here because it will change the logic, so I left a TODO so I don't forget the next time I'm working in this area.
apps/vscode/src/host/hooks.ts
Outdated
| const result = await vscode.commands.executeCommand<hooks.StatementRange>( | ||
| "vscode.executeStatementRangeProvider", | ||
| document.uri, | ||
| adjustedPosition(vdoc.language, position) | ||
| ); | ||
| return { range: unadjustedRange(vdoc.language, result.range), code: result.code }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This set of changes in this file breaks the statement range provider in source mode. We need to use the code as it was before the change, to propagate the statement range through the virtual doc. We can't call vscode.executeStatementRangeProvider on the .qmd but instead must call it on the vdoc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the catch. My mistake, that was not the change I intended to make. I meant to only inline the body of the getStatementRange function, but accidentally removed the withVirtualDoc code without noticing.
I've updated the change to be what I intended.
I intended to inline the function to avoid some confusion I was having coming from:
- Ironically, that the function was defined outside the context of the arrow function inside
withVirtualDoc. It being its own separate function, it required some extra investigation to realize that the command will be executed in a virtual doc, while I was looking into how we pass things around in the code base. - The code being differently factored than
provideHelpTopicdespite both consisting of calling a command.
juliasilge
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Things here reflect about what I understand to be the facts on the ground here.
Co-authored-by: Julia Silge <[email protected]>
Here are some added inline docs, minor refactors, and a large overview markdown file that links to various key places in the code and gives some examples of how parts of the extension communicate with eachother. The inline docs and minor refactors mostly aim to address places I have been getting stuck/lost repeatedly while navigating code over the past few months.
I did this to help me get re-oriented, and to be able to answer questions about how to architect #825 and future features that involve "piping things through" (sharing functionality between the Source Editor and Visual Editor by communicating from the Visual Editor to the host and LSP in a consistent way).