Use of project dir and new deployment selector for subdirectory support w/ select configuration support #1946
Conversation
| extensions/vscode/dist | ||
| extensions/vscode/node_modules | ||
| extensions/vscode/webviews/homeView/node_modules | ||
| extensions/vscode/webviews/homeView/dist |
There was a problem hiding this comment.
Changes have been replicated into different PR, should clean up after some merges. #1948
| @@ -185,7 +185,7 @@ | |||
| "category": "Posit Publisher" | |||
| }, | |||
| { | |||
| "command": "posit.publisher.homeView.selectConfigForDeployment", | |||
| "command": "posit.publisher.homeView.showSelectConfigForDeployment", | |||
There was a problem hiding this comment.
Change made to remove confusion (mostly my own).
| const response = await api.configurations.getAll(); | ||
| const response = await api.configurations.getAll({ | ||
| dir: activeDeployment.projectDir, | ||
| }); |
There was a problem hiding this comment.
Only can select configurations from the same location as the deployment file.
| const inspectResponse = await api.configurations.inspect(python); | ||
| const inspectResponse = await api.configurations.inspect(python, { | ||
| dir: activeDeployment.projectDir, | ||
| }); |
There was a problem hiding this comment.
This is selecting a config for an existing deployment, so we have to limit choices to the configs which are present in that particular project dir where the entrypoint was found.
| @@ -428,7 +432,7 @@ export async function selectConfig( | |||
| ) { | |||
| return ( | |||
| config.configurationName === | |||
| state.data.existingConfigurationName.label | |||
| state.data.existingConfigurationName.detail | |||
There was a problem hiding this comment.
configuration name is in detail, not sure why old code was working.
| @@ -20,7 +20,6 @@ export enum HostToWebviewMessageType { | |||
| PUBLISH_FINISH_SUCCESS = "publishFinishSuccess", | |||
| PUBLISH_FINISH_FAILURE = "publishFinishFailure", | |||
| UPDATE_CONTENTRECORD_SELECTION = "updateContentRecordSelection", | |||
| UPDATE_CONFIG_SELECTION = "updateConfigSelection", | |||
There was a problem hiding this comment.
We no longer update the config selection. The config is ALWAYS determined by what configuration name is present within the contentRecord, which is read from the file. If we want to change it, we patch the deployment, thus updating the file and causing a refresh, where the new configuration name is picked up.
| deploymentName: string; | ||
| projectDir: string; | ||
| configurationName?: string; | ||
| deploymentPath: string; |
There was a problem hiding this comment.
The deploymentPath value is guaranteed to be unique across the user's installation. This is set by the agent while reading the file system. Once we match up to the applicable contentRecord, we then use information within it to find the configuration and projectDir, thus switching back to a relative access which works for all users, even if sharing files.
| private updateWebViewViewConfigurations( | ||
| deploymentSelector: DeploymentSelector | null, | ||
| ) { | ||
| private updateWebViewViewConfigurations() { |
There was a problem hiding this comment.
No longer need to select the current configuration, since it is always derived out of the current contentRecord. Removing this also helped eliminate some race conditions I encountered.
| this.webviewConduit.sendMsg({ | ||
| kind: HostToWebviewMessageType.REFRESH_CONFIG_DATA, | ||
| content: { | ||
| configurations: this.configs, | ||
| configurationsInError: this.configsInError, | ||
| deploymentSelected: deploymentSelector, |
| savedState.configurationName, | ||
| savedState.projectDir, | ||
| ); | ||
| return this.getConfigBySelector(savedState); |
There was a problem hiding this comment.
Things became a lot simpler once we were able to switch to the new strategy of the simple selector for the contentRecord, which then determined the active config.
| return this.contentRecords.find( | ||
| (d) => d.deploymentName === name && d.projectDir === projectDir, | ||
| (d) => d.deploymentPath === selector.deploymentPath, |
There was a problem hiding this comment.
This becomes a unique match, due to the deploymentPath (added by the backend agent and not present in the deployment file) containing the full system path.
| @@ -700,7 +690,7 @@ export class HomeViewProvider implements WebviewViewProvider, Disposable { | |||
| } | |||
| } | |||
|
|
|||
| private async propogateDeploymentSelection( | |||
| private async propagateDeploymentSelection( | |||
There was a problem hiding this comment.
fixed spelling of propagate.
| @@ -67,8 +66,6 @@ const onMessageFromHost = (msg: HostToWebviewMessage): void => { | |||
| return onPublishFinishFailureMsg(msg); | |||
| case HostToWebviewMessageType.UPDATE_CONTENTRECORD_SELECTION: | |||
| return onUpdateContentRecordSelectionMsg(msg); | |||
| case HostToWebviewMessageType.UPDATE_CONFIG_SELECTION: | |||
| return onUpdateConfigSelectionMsg(msg); | |||
| } else if (name === null) { | ||
| if (msg.content.deploymentSelected) { | ||
| home.updateSelectedContentRecordBySelector(msg.content.deploymentSelected); | ||
| } else { |
sagerb
requested review from
dotNomad,
kgartland-rstudio,
mmarchetti and
tdstein
as code owners
Intent
projectDirfor "Select Active Configuration"Resolves #1944
Resolves #1853
Type of Change
Approach
Automated Tests
Directions for Reviewers
Checklist