Skip to content

Commit

Permalink
chore: address code review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Scott Dover <[email protected]>
  • Loading branch information
Scott Dover authored and Scott Dover committed Nov 8, 2024
1 parent ae03a77 commit df87733
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 25 deletions.
87 changes: 72 additions & 15 deletions client/src/components/ContentNavigator/ContentDataProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
FileType,
Position,
ProviderResult,
TabInputNotebook,
TabInputText,
TextDocument,
TextDocumentContentProvider,
ThemeIcon,
Expand Down Expand Up @@ -52,7 +54,7 @@ import {
FileManipulationEvent,
} from "./types";
import {
getEditorTabForItem,
getEditorTabsForItem,
getFileStatement,
isContainer as getIsContainer,
} from "./utils";
Expand Down Expand Up @@ -285,23 +287,62 @@ class ContentDataProvider
name: string,
): Promise<Uri | undefined> {
const closing = closeFileIfOpen(item);
if (!(await closing)) {
const removedTabUris = await closing;
if (!removedTabUris) {
return;
}

const newItem = await this.model.renameResource(item, name);
if (newItem) {
const newUri = newItem.vscUri;
if (closing !== true) {
// File was open before rename, so re-open it
commands.executeCommand("vscode.open", newUri);
}
if (!newItem) {
return;
}

const newUri = newItem.vscUri;
const oldUriToNewUriMap = [[item.vscUri, newUri]];
const newItemIsContainer = getIsContainer(newItem);
if (closing !== true && !newItemIsContainer) {
commands.executeCommand("vscode.open", newUri);
}
const urisToOpen = getPreviouslyOpenedChildItems(
await this.getChildren(newItem),
);
urisToOpen.forEach(([, newUri]) =>
commands.executeCommand("vscode.open", newUri),
);
oldUriToNewUriMap.push(...urisToOpen);
oldUriToNewUriMap.forEach(([uri, newUri]) =>
this._onDidManipulateFile.fire({
type: "rename",
uri: item.vscUri,
uri,
newUri,
});
return newUri;
}),
);
return newUri;

function getPreviouslyOpenedChildItems(childItems: ContentItem[]) {
const loadChildItems = closing !== true && newItemIsContainer;
if (!Array.isArray(removedTabUris) || !loadChildItems) {
return [];
}
// Here's where things get a little weird. When we rename folders in
// sas content, we _don't_ close those files. It doesn't matter since
// their path isn't hierarchical. In sas file system, the path is hierarchical,
// thus we need to re-open all the closed files. This does that by getting
// children and comparing the removedTabUris
const filteredChildItems = childItems
.map((childItem) => {
const matchingUri = removedTabUris.find((uri) =>
uri.path.endsWith(childItem.name),
);
if (!matchingUri) {
return;
}

return [matchingUri, childItem.vscUri];
})
.filter((exists) => exists);

return filteredChildItems;
}
}

Expand Down Expand Up @@ -697,10 +738,26 @@ class ContentDataProvider

export default ContentDataProvider;

const closeFileIfOpen = (item: ContentItem) => {
const tab = getEditorTabForItem(item);
if (tab) {
return window.tabGroups.close(tab);
const closeFileIfOpen = (item: ContentItem): Promise<Uri[]> | boolean => {
const tabs = getEditorTabsForItem(item);
if (tabs.length > 0) {
return new Promise((resolve, reject) => {
Promise.all(tabs.map((tab) => window.tabGroups.close(tab)))
.then(() =>
resolve(
tabs
.map(
(tab) =>
(tab.input instanceof TabInputText ||
tab.input instanceof TabInputNotebook) &&
tab.input.uri,
)
.filter((exists) => exists),
),
)
.catch(reject);
});
}

return true;
};
12 changes: 5 additions & 7 deletions client/src/components/ContentNavigator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,16 @@ const folderValidator = (

class ContentNavigator implements SubscriptionProvider {
private contentDataProvider: ContentDataProvider;
private contentModel: ContentModel;
private sourceType: ContentNavigatorConfig["sourceType"];
private treeIdentifier: ContentNavigatorConfig["treeIdentifier"];

get contentModel() {
return new ContentModel(this.contentAdapterForConnectionType());
}

constructor(context: ExtensionContext, config: ContentNavigatorConfig) {
this.sourceType = config.sourceType;
this.treeIdentifier = config.treeIdentifier;
this.contentModel = new ContentModel(
this.contentAdapterForConnectionType(),
);
this.contentDataProvider = new ContentDataProvider(
this.contentModel,
context.extensionUri,
Expand Down Expand Up @@ -400,9 +400,7 @@ class ContentNavigator implements SubscriptionProvider {
if (event.affectsConfiguration("SAS.connectionProfiles")) {
const endpoint = this.viyaEndpoint();
this.collapseAllContent();
this.contentDataProvider.useModel(
new ContentModel(this.contentAdapterForConnectionType()),
);
this.contentDataProvider.useModel(this.contentModel);
if (endpoint) {
await this.contentDataProvider.connect(endpoint);
} else {
Expand Down
6 changes: 3 additions & 3 deletions client/src/components/ContentNavigator/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ export const createStaticFolder = (
],
});

export const getEditorTabForItem = (item: ContentItem) => {
export const getEditorTabsForItem = (item: ContentItem) => {
const fileUri = item.vscUri;
const tabs: Tab[] = window.tabGroups.all.map((tg) => tg.tabs).flat();
return tabs.find(
return tabs.filter(
(tab) =>
(tab.input instanceof TabInputText ||
tab.input instanceof TabInputNotebook) &&
tab.input.uri.query === fileUri.query, // compare the file id
tab.input.uri.query.includes(fileUri.query), // compare the file id
);
};
5 changes: 5 additions & 0 deletions client/src/connection/rest/RestSASServerAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ class RestSASServerAdapter implements ContentAdapter {
});

const contentItem = this.filePropertiesToContentItem(response.data);
this.updateFileMetadata(
this.trimComputePrefix(contentItem.uri),
response,
);

if (buffer) {
await this.updateContentOfItemAtPath(
Expand Down Expand Up @@ -249,6 +253,7 @@ class RestSASServerAdapter implements ContentAdapter {
}

private async getContentOfItemAtPath(path: string) {
await this.setup();
const response = await this.fileSystemApi.getFileContentFromSystem(
{
sessionId: this.sessionId,
Expand Down

0 comments on commit df87733

Please sign in to comment.