-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add document tracker to set activeFileEntrypoint
context
#1883
Changes from 18 commits
0f4ddc0
db852fa
d2d9280
c0c142d
6df72c3
3dd8867
b5bdee6
1693223
a467ef9
30f7a3e
9a3f6ae
1113b9a
0f5ca1e
a7d39dc
10e0121
1a02a5d
df6aca0
cf1a3e1
69d7a74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -251,9 +251,22 @@ | |
"title": "View Content Log on Connect", | ||
"icon": "$(output)", | ||
"category": "Posit Publisher" | ||
}, | ||
{ | ||
"command": "posit.publisher.deployWithEntrypoint", | ||
"title": "Deploy with this Entrypoint", | ||
"icon": "$(cloud-upload)", | ||
"category": "Posit Publisher" | ||
} | ||
], | ||
"menus": { | ||
"editor/title": [ | ||
{ | ||
"command": "posit.publisher.deployWithEntrypoint", | ||
"group": "navigation", | ||
"when": "posit.publish.activeFileEntrypoint == true" | ||
} | ||
], | ||
"view/title": [ | ||
{ | ||
"command": "posit.publisher.configurations.add", | ||
|
@@ -685,6 +698,7 @@ | |
"eventsource": "^2.0.2", | ||
"get-port": "5.1.1", | ||
"mutexify": "^1.4.0", | ||
"retry": "^0.13.1" | ||
"retry": "^0.13.1", | ||
"vscode-uri": "^3.0.8" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,214 @@ | ||
// Copyright (C) 2023 by Posit Software, PBC. | ||
|
||
import { | ||
Disposable, | ||
TextDocument, | ||
TextEditor, | ||
commands, | ||
window, | ||
workspace, | ||
} from "vscode"; | ||
import { Utils as uriUtils } from "vscode-uri"; | ||
|
||
import { useApi } from "src/api"; | ||
import { Contexts } from "src/constants"; | ||
import { getPythonInterpreterPath } from "src/utils/config"; | ||
import { isActiveDocument, relativeDir } from "src/utils/files"; | ||
import { hasKnownContentType } from "src/utils/inspect"; | ||
import { getSummaryStringFromError } from "src/utils/errors"; | ||
|
||
/** | ||
* Determines if a text document is an entrypoint file. | ||
* | ||
* @param document The text document to inspect | ||
* @returns If the text document is an entrypoint | ||
*/ | ||
async function isDocumentEntrypoint(document: TextDocument): Promise<boolean> { | ||
const dir = relativeDir(document.uri); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was quite tricky digging into what VSCode could do for us and what else is out there. I broke it out into its own utils function so we can get a |
||
// If the file is outside the workspace, it cannot be an entrypoint | ||
if (dir === undefined) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we don't want anything but There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, but I don't think we need to per our implementation plan. |
||
return false; | ||
} | ||
|
||
try { | ||
const api = await useApi(); | ||
const python = await getPythonInterpreterPath(); | ||
|
||
const response = await api.configurations.inspect(python, { | ||
dir: dir, | ||
entrypoint: uriUtils.basename(document.uri), | ||
}); | ||
|
||
return hasKnownContentType(response.data); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm only marking a document as an entrypoint if any of the inspections have a |
||
} catch (error: unknown) { | ||
const summary = getSummaryStringFromError( | ||
"entrypointTracker::isDocumentEntrypoint", | ||
error, | ||
); | ||
window.showInformationMessage(summary); | ||
return false; | ||
} | ||
} | ||
|
||
/** | ||
* Tracks whether a document is an entrypoint file and sets extension context. | ||
*/ | ||
export class TrackedEntrypointDocument { | ||
readonly document: TextDocument; | ||
private isEntrypoint: boolean; | ||
|
||
private requiresUpdate: boolean = false; | ||
|
||
private constructor(document: TextDocument, isEntrypoint: boolean) { | ||
this.document = document; | ||
this.isEntrypoint = isEntrypoint; | ||
} | ||
|
||
static async create(document: TextDocument) { | ||
const isEntrypoint = await isDocumentEntrypoint(document); | ||
return new TrackedEntrypointDocument(document, isEntrypoint); | ||
} | ||
|
||
/** | ||
* Sets the file entrypoint context with this as the active file. | ||
* @param options Options for the activation | ||
* @param options.forceUpdate Whether to force the entrypoint to update | ||
*/ | ||
async activate(options?: { forceUpdate?: boolean }) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The API calls only happen when we create a This occurs when the document is saved, or the document is re-opened.
|
||
// change based on if entrypoint | ||
if (options?.forceUpdate) { | ||
await this.update(); | ||
} else if (this.requiresUpdate) { | ||
await this.update(); | ||
} | ||
|
||
commands.executeCommand( | ||
"setContext", | ||
Contexts.ActiveFileEntrypoint, | ||
this.isEntrypoint, | ||
); | ||
} | ||
|
||
/** | ||
* Updates the entrypoint next time the document is activated. | ||
*/ | ||
updateNextActivate() { | ||
this.requiresUpdate = true; | ||
} | ||
|
||
/** | ||
* Updates whether or not the document is an entrypoint file. | ||
*/ | ||
private async update() { | ||
this.requiresUpdate = false; | ||
this.isEntrypoint = await isDocumentEntrypoint(this.document); | ||
} | ||
} | ||
|
||
/** | ||
* Tracks active documents and assists in determining extension context. | ||
*/ | ||
export class DocumentTracker implements Disposable { | ||
private disposable: Disposable; | ||
|
||
private readonly documents = new Map< | ||
TextDocument, | ||
TrackedEntrypointDocument | ||
>(); | ||
|
||
constructor() { | ||
this.disposable = Disposable.from( | ||
window.onDidChangeActiveTextEditor(this.onActiveTextEditorChanged, this), | ||
workspace.onDidCloseTextDocument(this.onTextDocumentClosed, this), | ||
workspace.onDidSaveTextDocument(this.onTextDocumentSaved, this), | ||
); | ||
|
||
// activate the initial active file | ||
this.onActiveTextEditorChanged(window.activeTextEditor); | ||
} | ||
|
||
dispose() { | ||
this.disposable.dispose(); | ||
this.documents.clear(); | ||
} | ||
|
||
/** | ||
* Starts tracking a document | ||
* @param document The document to track | ||
* @returns The TrackedEntrypointDocument created | ||
*/ | ||
async addDocument(document: TextDocument) { | ||
const entrypoint = await TrackedEntrypointDocument.create(document); | ||
this.documents.set(document, entrypoint); | ||
return entrypoint; | ||
} | ||
|
||
/** | ||
* Stops tracking a document | ||
* @param document The document to stop tracking | ||
*/ | ||
removeDocument(document: TextDocument) { | ||
this.documents.delete(document); | ||
} | ||
|
||
/** | ||
* Listener function for changes to the active text editor. | ||
* | ||
* Adds new documents to the tracker, and activates the associated | ||
* TrackedEntrypointDocument | ||
* @param editor The active text editor | ||
*/ | ||
async onActiveTextEditorChanged(editor: TextEditor | undefined) { | ||
if (editor === undefined) { | ||
commands.executeCommand( | ||
"setContext", | ||
Contexts.ActiveFileEntrypoint, | ||
undefined, | ||
); | ||
return; | ||
} | ||
|
||
let tracked = this.documents.get(editor.document); | ||
|
||
if (tracked === undefined) { | ||
tracked = await this.addDocument(editor.document); | ||
} | ||
|
||
tracked.activate(); | ||
} | ||
|
||
/** | ||
* Listener function for the closing of a text document. | ||
* Stops the document from being tracked. | ||
* | ||
* @param document The closed document | ||
*/ | ||
onTextDocumentClosed(document: TextDocument) { | ||
this.removeDocument(document); | ||
} | ||
Comment on lines
+186
to
+188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documents are removed when closed since another editor could potentially change the document. This way when a document is re-opened we recheck just in case. |
||
|
||
/** | ||
* Listener function for the saving of a text document. | ||
* Triggers the document to update next time it is activated. | ||
* | ||
* @param document The saved document | ||
*/ | ||
async onTextDocumentSaved(document: TextDocument) { | ||
const tracked = this.documents.get(document); | ||
|
||
if (tracked) { | ||
if (isActiveDocument(document)) { | ||
tracked.activate({ forceUpdate: true }); | ||
} else { | ||
tracked.updateNextActivate(); | ||
} | ||
return; | ||
} | ||
|
||
// Track the untracked document | ||
const newTracked = await this.addDocument(document); | ||
if (isActiveDocument(document)) { | ||
newTracked.activate(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ import { EventStream } from "src/events"; | |
import { HomeViewProvider } from "src/views/homeView"; | ||
import { WatcherManager } from "src/watchers"; | ||
import { Commands } from "src/constants"; | ||
import { DocumentTracker } from "./entrypointTracker"; | ||
|
||
const STATE_CONTEXT = "posit.publish.state"; | ||
|
||
|
@@ -100,6 +101,14 @@ export async function activate(context: ExtensionContext) { | |
), | ||
); | ||
setStateContext(PositPublishState.initialized); | ||
|
||
context.subscriptions.push( | ||
new DocumentTracker(), | ||
commands.registerCommand(Commands.DeployWithEntrypoint, () => { | ||
dotNomad marked this conversation as resolved.
Show resolved
Hide resolved
|
||
commands.executeCommand(Commands.HomeView.Focus); | ||
console.log("'Deploy with this Entrypoint' button hit!"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently the button only console logs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Easily built upon! Great! |
||
}), | ||
); | ||
} | ||
|
||
// This method is called when your extension is deactivated | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,9 +8,11 @@ import { | |
workspace, | ||
window, | ||
commands, | ||
TextDocument, | ||
} from "vscode"; | ||
import { Utils as uriUtils } from "vscode-uri"; | ||
|
||
import { ContentRecordFile } from "../api"; | ||
import { ContentRecordFile } from "src/api"; | ||
|
||
export async function fileExists(fileUri: Uri): Promise<boolean> { | ||
try { | ||
|
@@ -231,3 +233,34 @@ export function splitFilesOnInclusion( | |
export function isRelativePathRoot(path: string): boolean { | ||
return path === "."; | ||
} | ||
|
||
export function isActiveDocument(document: TextDocument): boolean { | ||
const editor = window.activeTextEditor; | ||
return editor?.document === document; | ||
} | ||
|
||
/** | ||
* Returns a VSCode workspace relative directory path for a given URI. | ||
* | ||
* @param uri The URI to get the relative dirname of | ||
* @returns A relative path `string` if the URI is in the workspace | ||
* @returns `undefined` if the URI is not in the workspace | ||
*/ | ||
export function relativeDir(uri: Uri): string | undefined { | ||
const workspaceFolder = workspace.getWorkspaceFolder(uri); | ||
|
||
if (!workspaceFolder) { | ||
// File is outside of the workspace | ||
return undefined; | ||
} | ||
|
||
const dirname = uriUtils.dirname(uri); | ||
|
||
// If the file is in the workspace root, return "." | ||
if (dirname.fsPath === workspaceFolder.uri.fsPath) { | ||
return "."; | ||
} | ||
Comment on lines
+260
to
+262
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That doesn't work for us, so here I check to see if the file system path is the same and return a |
||
|
||
// Otherwise, return the relative path VSCode expects | ||
return workspace.asRelativePath(dirname); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// Copyright (C) 2023 by Posit Software, PBC. | ||
|
||
import { ConfigurationInspectionResult, ContentType } from "src/api"; | ||
|
||
/** | ||
* Determines if some of the inspections have a known content type. | ||
* | ||
* @param inspections The results of an /api/inspect request | ||
* @returns boolean - if any of the inspections have a known content type | ||
*/ | ||
export function hasKnownContentType( | ||
inspections: ConfigurationInspectionResult[], | ||
) { | ||
return inspections.some( | ||
(inspection) => inspection.configuration.type !== ContentType.UNKNOWN, | ||
); | ||
} |
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.
Using a
cloud-upload
icon to avoid needing to deal with a PNG, or getting an SVG into a font for release.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.
As discussed, do we just want to use the PNG that we have for the old publisher icon, so we can replace both at once to the new one?
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.
I tested this out and it does work, and doesn't look too bad even though it is being scaled down from
450x450
to16x16
However since it is a
png
it doesn't get colored using the active theme. So even if I changed it to be white/gray/etc it would look incorrect depending on the theme used.Personally I'd prefer to avoid the non-native look until we get that fixed.
If others disagree, it can be easily changed while I'm out with the below example: