Auto Preview Support #31
Conversation
…on settings allowing the user to control behavior via autoPreview and autoPreviewToSide
| @@ -3,7 +3,7 @@ | |||
| "publisher": "iliazeus", | |||
| "displayName": "ANSI Colors", | |||
| "description": "ANSI color styling for text documents", | |||
| "version": "1.1.7", | |||
| "version": "1.1.8", | |||
There was a problem hiding this comment.
I'd consider it a minor-version-worthy change.
| "version": "1.1.8", | |
| "version": "1.2.0", |
| @@ -26,7 +26,9 @@ | |||
| "colors", | |||
| "logs" | |||
| ], | |||
| "activationEvents": [], | |||
| "activationEvents": [ | |||
| "onStartupFinished" | |||
There was a problem hiding this comment.
I specifically do not want the extension activating on every startup. Is this activation event really needed?
| "properties": { | ||
| "iliazeus.vscode-ansi.autoPreview": { | ||
| "type": "boolean", | ||
| "default": true, |
There was a problem hiding this comment.
| "default": true, | |
| "default": false, |
We should not break the existing behavior.
| "iliazeus.vscode-ansi.autoPreviewToSide": { | ||
| "type": "boolean", | ||
| "default": false, | ||
| "description": "If Auto Preview is set to True: Automatically open the ANSI Text Preview to the side. Warning: This can cause undesirable behavior!" |
There was a problem hiding this comment.
Can you please explain what this undesirable behavior is? Maybe we can fix it, or just don't include this specific option.
| "dependencies": {} | ||
| } | ||
| "dependencies": { | ||
| "yarn": "^1.22.22" |
There was a problem hiding this comment.
This repo assumes yarn is installed globally, same as npm in most (current) node projects. This explicit dependency should be removed.
| const showPretty = async (forcePreview: boolean = false, options: TextDocumentShowOptions = { viewColumn: ViewColumn.Active}) => { | ||
| if (forcePreview || workspace.getConfiguration(`${extensionId}`).autoPreview) { | ||
| const editor = window.activeTextEditor | ||
| if (editor && editor.document && editor.document.languageId === 'ansi' && editor.document.uri.scheme === 'file') { |
There was a problem hiding this comment.
I'd change the logic from "only file scheme" to "every non-ours scheme". This will make it integrate better with other extensions, e.g. with the "view file at revision" feature of GitLens.
| @@ -29,23 +30,25 @@ export async function activate(context: ExtensionContext): Promise<void> { | |||
| workspace.registerTextDocumentContentProvider(PrettyAnsiContentProvider.scheme, prettyAnsiContentProvider) | |||
| ); | |||
|
|
|||
| const showPretty = async (options?: TextDocumentShowOptions) => { | |||
| const actualUri = window.activeTextEditor?.document.uri; | |||
| const showPretty = async (forcePreview: boolean = false, options: TextDocumentShowOptions = { viewColumn: ViewColumn.Active}) => { | |||
There was a problem hiding this comment.
I am against positional boolean parameters, because they are difficult to understand at call site (i.e. what does true in doThing(true) even mean?)
I'd change this to something more like showPretty({ viewColumn: ..., forcePreview: true }).
| } | ||
|
|
||
| const providerUri = PrettyAnsiContentProvider.toProviderUri(actualUri); | ||
| const autoPreviewToSide = workspace.getConfiguration(`${extensionId}`).autoPreviewToSide; |
There was a problem hiding this comment.
| const autoPreviewToSide = workspace.getConfiguration(`${extensionId}`).autoPreviewToSide; | |
| const autoPreviewToSide = workspace.getConfiguration(extensionId).autoPreviewToSide; |
This template literal is unnecessary.
| window.onDidChangeActiveTextEditor((textEditor: TextEditor | undefined) => { | ||
| showPretty(); | ||
| }); | ||
|
|
||
| showPretty(); |
There was a problem hiding this comment.
I don't quite understand why we need this.
Solves #26