Add command palette support and export from a file #3548
Conversation
| <?xml version="1.0" encoding="utf-8"?> |
There was a problem hiding this comment.
Forgot to mention we got new icons from Cherry. #ByDesign
| @inject(ILogger) private logger: ILogger) { | ||
| } | ||
|
|
||
| public dispose() { |
There was a problem hiding this comment.
Why is the dispose needed if it's a noop? #WontFix
There was a problem hiding this comment.
I guess it isn't. All of the INotebook thingies implement Idisposable, but this one doesn't need to. Hmm. Well it would be simpler without it. I'll remove it.
In reply to: 238828207 [](ancestors = 238828207)
There was a problem hiding this comment.
Actually I think I'll leave it for consistency's sake. It does no harm as nobody adds it to the disposable registry, but then this being the only one that isn't disposable seems odd.
In reply to: 238829492 [](ancestors = 238829492,238828207)
| ]; | ||
| } else { | ||
| // Just a single markdown cell | ||
| return [generateMarkdownCell(code, file, line)]; |
There was a problem hiding this comment.
Using code here not copy means that one code path is using unix line endings and another code path is not. #Pending
There was a problem hiding this comment.
| const firstNonMarkdown = splitMarkdown ? split.findIndex((l: string) => l.trim().length > 0 && !l.trim().startsWith('#')) : -1; | ||
| if (firstNonMarkdown >= 0) { | ||
| return [ | ||
| generateMarkdownCell(split.slice(0, firstNonMarkdown).join('\n'), file, line), |
There was a problem hiding this comment.
Could these just use a string[] so you don't need to do split / join / split on the same strings? #Pending
There was a problem hiding this comment.
I guess I could have overloads. generateCell/generateMarkdownCell are called externally with just strings.
In reply to: 238832077 [](ancestors = 238832077)
There was a problem hiding this comment.
actually I'm wrong, they're only called internally.
In reply to: 238832588 [](ancestors = 238832588,238832077)
There was a problem hiding this comment.
not a big deal, it's not like it's an expensive operation. Just kinda scanned funny here.
In reply to: 238832588 [](ancestors = 238832588,238832077)
| } else { | ||
| const activeEditor = this.documentManager.activeTextEditor; | ||
| if (activeEditor && activeEditor.document.languageId === PYTHON_LANGUAGE) { | ||
| await this.exportFileAndOutput(activeEditor.document.fileName); |
There was a problem hiding this comment.
Copy paste error? Should just be export file. #Pending
There was a problem hiding this comment.
You are right. That should be exportFile. I need to update the unit test.
In reply to: 238834387 [](ancestors = 238834387)
For #3476 - Add command palette support
Also support exporting a file with and without running all of the cells in it.
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed)