Skip to content

Infer doc URI based on doc name for the New File command#1741

Merged
isc-kcressma merged 2 commits intointersystems-community:masterfrom
isc-kcressma:fix-1734
Apr 1, 2026
Merged

Infer doc URI based on doc name for the New File command#1741
isc-kcressma merged 2 commits intointersystems-community:masterfrom
isc-kcressma:fix-1734

Conversation

@isc-kcressma
Copy link
Copy Markdown
Collaborator

This PR fixes #1734

// Use the inferred URI directly without prompting
clsUri = inferredUri;
} else {
// Fall back to objectscript.export settings and prompt the user
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest extracting the prompt logic into a helper function to make the control flow clearer and easier to follow.

clsUri = inferDocUri(`${cls}.cls`, wsFolder) ?? promptDocUri(...);

*/
export function inferDocUri(docName: string, wsFolder: vscode.WorkspaceFolder): vscode.Uri | undefined {
const exts = [".cls", ".mac", ".int", ".inc"];
const docExt = docName.slice(docName.lastIndexOf("."));
Copy link
Copy Markdown
Contributor

@isc-bsaviano isc-bsaviano Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can just do slice(-4)

// Convert the document name to a file path and prepend the prefix
const docNamePath = `${docNameNoExt.replaceAll(".", "/")}${docExt}`;
const inferredPath = `${bestPathPrefix}${docNamePath}`;
return wsFolder.uri.with({ path: inferredPath });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These last three lines can be condensed into one by removing the intermediate const declarations. It's a purely stylistic change you can reject though.

@isc-bsaviano
Copy link
Copy Markdown
Contributor

Nice first PR!

// Count how many leading package segments the indexed doc shares with the target
const indexPkgSegments = indexDocName.slice(0, -4).split(".").slice(0, -1);
let matchLen = 0;
for (let i = 0; i < Math.min(docPkgSegments.length, indexPkgSegments.length); i++) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need both matchLen and i. I would remove matchLen and increment i instead.

indexDocFullPath = indexDocFullPath.slice(0, -3) + indexDocFullPath.slice(-3).toLowerCase();

if (!indexDocFullPath.endsWith(indexDocNamePath)) return;
const indexPathPrefix = indexDocFullPath.slice(0, -indexDocNamePath.length + 1);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of lines 507–514 appears to be computing indexPathPrefix—the directory prefix for a .cls file. This logic seems broadly reusable, and it might be beneficial to extract it into a helper function.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to leave it as is given that it's only a few lines, is kind of an abstract operation, has cases where the closure needs to exit, and I think currently would be only be reusable in the inferDocName method

@isc-bsaviano
Copy link
Copy Markdown
Contributor

Nice work Keith!

@isc-kcressma isc-kcressma merged commit f0e8c0b into intersystems-community:master Apr 1, 2026
5 checks passed
@isc-kcressma isc-kcressma deleted the fix-1734 branch April 1, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve generation of URIs in client-side workspace folders for New File commands

3 participants