Infer doc URI based on doc name for the New File command#1741
Infer doc URI based on doc name for the New File command#1741isc-kcressma merged 2 commits intointersystems-community:masterfrom
Conversation
src/commands/newFile.ts
Outdated
| // Use the inferred URI directly without prompting | ||
| clsUri = inferredUri; | ||
| } else { | ||
| // Fall back to objectscript.export settings and prompt the user |
There was a problem hiding this comment.
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(...);
src/utils/documentIndex.ts
Outdated
| */ | ||
| export function inferDocUri(docName: string, wsFolder: vscode.WorkspaceFolder): vscode.Uri | undefined { | ||
| const exts = [".cls", ".mac", ".int", ".inc"]; | ||
| const docExt = docName.slice(docName.lastIndexOf(".")); |
There was a problem hiding this comment.
Can just do slice(-4)
src/utils/documentIndex.ts
Outdated
| // 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 }); |
There was a problem hiding this comment.
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.
|
Nice first PR! |
src/utils/documentIndex.ts
Outdated
| // 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++) { |
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
|
Nice work Keith! |
This PR fixes #1734