Skip to content

Commit 9939ef9

Browse files
committed
Respect file path casing in extension commands
Per https://nodejs.org/en/docs/guides/working-with-different-filesystems we should really just keep the file path we're given, no "normalizing" to be done especially since it is wrong to infer from the OS if the filesystem is case-sensitive or not. Gone are the days where Windows and macOS could be presumed case-insensitive, they now both support and are often found with with case-sensitive filesystems. Plus, there was really no reason to be doing this in the first place. Tested interactively, this works just fine since the VS Code APIs we're using handle case-insensitivity for us. Resolves #2960.
1 parent 46f1584 commit 9939ef9

File tree

2 files changed

+56
-108
lines changed

2 files changed

+56
-108
lines changed

src/features/ExtensionCommands.ts

+46-101
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
import * as os from "os";
54
import * as path from "path";
65
import * as vscode from "vscode";
76
import {
@@ -141,6 +140,7 @@ export interface IStatusBarMessageDetails {
141140
message: string;
142141
timeout?: number;
143142
}
143+
144144
interface IInvokeRegisteredEditorCommandParameter {
145145
commandName: string;
146146
}
@@ -161,11 +161,8 @@ export class ExtensionCommandsFeature extends LanguageClientConsumer {
161161

162162
vscode.commands.registerCommand("PowerShell.InvokeRegisteredEditorCommand",
163163
async (param: IInvokeRegisteredEditorCommandParameter) => {
164-
if (this.extensionCommands.length === 0) {
165-
return;
166-
}
167-
168-
const commandToExecute = this.extensionCommands.find((x) => x.name === param.commandName);
164+
const commandToExecute = this.extensionCommands.find(
165+
(x) => x.name === param.commandName);
169166

170167
if (commandToExecute) {
171168
await this.languageClient?.sendRequest(
@@ -219,7 +216,8 @@ export class ExtensionCommandsFeature extends LanguageClientConsumer {
219216

220217
this.languageClient.onRequest(
221218
NewFileRequestType,
222-
// TODO: Shouldn't this use the file path?
219+
// NOTE: The VS Code API does not support naming a file as it's
220+
// opened, only when it's saved. Hence the argument is not used.
223221
(_filePath) => this.newFile()),
224222

225223
this.languageClient.onRequest(
@@ -300,7 +298,7 @@ export class ExtensionCommandsFeature extends LanguageClientConsumer {
300298

301299
const selectedCommand = await vscode.window.showQuickPick(
302300
quickPickItems,
303-
{ placeHolder: "Select a command" });
301+
{ placeHolder: "Select a command..." });
304302
return this.onCommandSelected(selectedCommand, client);
305303
}
306304

@@ -364,15 +362,16 @@ export class ExtensionCommandsFeature extends LanguageClientConsumer {
364362
}
365363

366364
private async openFile(openFileDetails: IOpenFileDetails): Promise<EditorOperationResponse> {
367-
const filePath = await this.normalizeFilePath(openFileDetails.filePath);
365+
const filePath = await this.resolveFilePathWithCwd(openFileDetails.filePath);
368366
const doc = await vscode.workspace.openTextDocument(filePath);
369367
await vscode.window.showTextDocument(doc, { preview: openFileDetails.preview });
370368
return EditorOperationResponse.Completed;
371369
}
372370

373371
private async closeFile(filePath: string): Promise<EditorOperationResponse> {
374-
if (this.findTextDocument(await this.normalizeFilePath(filePath))) {
375-
const doc = await vscode.workspace.openTextDocument(filePath);
372+
filePath = await this.resolveFilePathWithCwd(filePath);
373+
const doc = vscode.workspace.textDocuments.find((x) => x.fileName === filePath);
374+
if (doc != undefined && !doc.isClosed) {
376375
await vscode.window.showTextDocument(doc);
377376
await vscode.commands.executeCommand("workbench.action.closeActiveEditor");
378377
}
@@ -384,142 +383,88 @@ export class ExtensionCommandsFeature extends LanguageClientConsumer {
384383
* @param saveFileDetails the object detailing the path of the file to save and optionally its new path to save to
385384
*/
386385
private async saveFile(saveFileDetails: ISaveFileDetails): Promise<EditorOperationResponse> {
387-
// Try to interpret the filepath as a URI, defaulting to "file://" if we don't succeed
386+
// Try to interpret the filePath as a URI, defaulting to "file://" if we don't succeed
388387
let currentFileUri: vscode.Uri;
389388
if (saveFileDetails.filePath.startsWith("untitled") || saveFileDetails.filePath.startsWith("file")) {
390389
currentFileUri = vscode.Uri.parse(saveFileDetails.filePath);
391390
} else {
392391
currentFileUri = vscode.Uri.file(saveFileDetails.filePath);
393392
}
394393

395-
let newFileAbsolutePath: string;
396-
switch (currentFileUri.scheme) {
397-
case "file": {
398-
// If the file to save can't be found, just complete the request
399-
if (!this.findTextDocument(await this.normalizeFilePath(currentFileUri.fsPath))) {
400-
void this.logger.writeAndShowError(`File to save not found: ${currentFileUri.fsPath}.`);
401-
return EditorOperationResponse.Completed;
402-
}
394+
// If the file to save can't be found, just complete the request
395+
const doc = vscode.workspace.textDocuments.find((x) => x.uri === currentFileUri);
396+
if (doc === undefined) {
397+
void this.logger.writeAndShowError(`File to save not found: ${currentFileUri.fsPath}`);
398+
return EditorOperationResponse.Completed;
399+
}
403400

401+
let newFilePath = saveFileDetails.newPath;
402+
if (currentFileUri.scheme === "file") {
404403
// If no newFile is given, just save the current file
405-
if (!saveFileDetails.newPath) {
406-
const doc = await vscode.workspace.openTextDocument(currentFileUri.fsPath);
404+
if (newFilePath === undefined) {
407405
if (doc.isDirty) {
408406
await doc.save();
409407
}
410408
return EditorOperationResponse.Completed;
411409
}
412410

413-
// Make sure we have an absolute path
414-
if (path.isAbsolute(saveFileDetails.newPath)) {
415-
newFileAbsolutePath = saveFileDetails.newPath;
416-
} else {
417-
// If not, interpret the path as relative to the current file
418-
newFileAbsolutePath = path.join(path.dirname(currentFileUri.fsPath), saveFileDetails.newPath);
411+
// Special case where we interpret a path as relative to the current
412+
// file, not the CWD!
413+
if (!path.isAbsolute(newFilePath)) {
414+
newFilePath = path.join(path.dirname(currentFileUri.fsPath), newFilePath);
419415
}
420-
break; }
421-
422-
case "untitled": {
416+
} else if (currentFileUri.scheme === "untitled") {
423417
// We need a new name to save an untitled file
424-
if (!saveFileDetails.newPath) {
425-
// TODO: Create a class handle vscode warnings and errors so we can warn easily
426-
// without logging
427-
void this.logger.writeAndShowWarning("Cannot save untitled file. Try SaveAs(\"path/to/file.ps1\") instead.");
418+
if (newFilePath === undefined) {
419+
void this.logger.writeAndShowError("Cannot save untitled file! Try SaveAs(\"path/to/file.ps1\") instead.");
428420
return EditorOperationResponse.Completed;
429421
}
430422

431-
// Make sure we have an absolute path
432-
if (path.isAbsolute(saveFileDetails.newPath)) {
433-
newFileAbsolutePath = saveFileDetails.newPath;
434-
} else {
435-
const cwd = await validateCwdSetting(this.logger);
436-
newFileAbsolutePath = path.join(cwd, saveFileDetails.newPath);
437-
}
438-
break; }
439-
440-
default: {
423+
newFilePath = await this.resolveFilePathWithCwd(newFilePath);
424+
} else {
441425
// Other URI schemes are not supported
442426
const msg = JSON.stringify(saveFileDetails, undefined, 2);
443-
this.logger.writeVerbose(
427+
void this.logger.writeAndShowError(
444428
`<${ExtensionCommandsFeature.name}>: Saving a document with scheme '${currentFileUri.scheme}' ` +
445-
`is currently unsupported. Message: '${msg}'`);
446-
return EditorOperationResponse.Completed; }
429+
`is currently unsupported. Message: '${msg}'`);
430+
return EditorOperationResponse.Completed;
447431
}
448432

449-
await this.saveDocumentContentToAbsolutePath(currentFileUri, newFileAbsolutePath);
433+
await this.saveFileAs(doc, newFilePath);
450434
return EditorOperationResponse.Completed;
451435

452436
}
453437

454438
/**
455439
* Take a document available to vscode at the given URI and save it to the given absolute path
456440
* @param documentUri the URI of the vscode document to save
457-
* @param destinationAbsolutePath the absolute path to save the document contents to
441+
* @param filePath the absolute path to save the document contents to
458442
*/
459-
private async saveDocumentContentToAbsolutePath(
460-
documentUri: vscode.Uri,
461-
destinationAbsolutePath: string): Promise<void> {
462-
// Retrieve the text out of the current document
463-
const oldDocument = await vscode.workspace.openTextDocument(documentUri);
464-
465-
// Write it to the new document path
443+
private async saveFileAs(doc: vscode.TextDocument, filePath: string): Promise<void> {
444+
// Write the old document's contents to the new document path
445+
const newFileUri = vscode.Uri.file(filePath);
466446
try {
467447
await vscode.workspace.fs.writeFile(
468-
vscode.Uri.file(destinationAbsolutePath),
469-
Buffer.from(oldDocument.getText()));
448+
newFileUri,
449+
Buffer.from(doc.getText()));
470450
} catch (err) {
471451
void this.logger.writeAndShowWarning(`<${ExtensionCommandsFeature.name}>: ` +
472-
`Unable to save file to path '${destinationAbsolutePath}': ${err}`);
452+
`Unable to save file to path '${filePath}': ${err}`);
473453
return;
474454
}
475455

476456
// Finally open the new document
477-
const newFileUri = vscode.Uri.file(destinationAbsolutePath);
478457
const newFile = await vscode.workspace.openTextDocument(newFileUri);
479458
await vscode.window.showTextDocument(newFile, { preview: true });
480459
}
481460

482-
private async normalizeFilePath(filePath: string): Promise<string> {
483-
const cwd = await validateCwdSetting(this.logger);
484-
const platform = os.platform();
485-
if (platform === "win32") {
486-
// Make sure the file path is absolute
487-
if (!path.win32.isAbsolute(filePath)) {
488-
filePath = path.win32.resolve(cwd, filePath);
489-
}
490-
491-
// Normalize file path case for comparison for Windows
492-
return filePath.toLowerCase();
493-
} else {
494-
// Make sure the file path is absolute
495-
if (!path.isAbsolute(filePath)) {
496-
filePath = path.resolve(cwd, filePath);
497-
}
498-
499-
// macOS is case-insensitive
500-
if (platform === "darwin") {
501-
filePath = filePath.toLowerCase();
502-
}
503-
504-
return filePath;
461+
// Resolve file path against user's CWD setting
462+
private async resolveFilePathWithCwd(filePath: string): Promise<string> {
463+
if (!path.isAbsolute(filePath)) {
464+
const cwd = await validateCwdSetting(this.logger);
465+
return path.resolve(cwd, filePath);
505466
}
506-
}
507-
508-
private findTextDocument(filePath: string): boolean {
509-
// since Windows and macOS are case-insensitive, we need to normalize them differently
510-
const canFind = vscode.workspace.textDocuments.find((doc) => {
511-
let docPath: string;
512-
const platform = os.platform();
513-
if (platform === "win32" || platform === "darwin") {
514-
// for Windows and macOS paths, they are normalized to be lowercase
515-
docPath = doc.fileName.toLowerCase();
516-
} else {
517-
docPath = doc.fileName;
518-
}
519-
return docPath === filePath;
520-
});
521-
522-
return canFind != null;
467+
return filePath;
523468
}
524469

525470
private setSelection(details: ISetSelectionRequestArguments): EditorOperationResponse {

src/session.ts

+10-7
Original file line numberDiff line numberDiff line change
@@ -444,10 +444,10 @@ export class SessionManager implements Middleware {
444444

445445
// Detect any setting changes that would affect the session.
446446
if (!this.suppressRestartPrompt && this.sessionStatus === SessionStatus.Running &&
447-
(settings.cwd.toLowerCase() !== this.sessionSettings.cwd.toLowerCase()
448-
|| settings.powerShellDefaultVersion.toLowerCase() !== this.sessionSettings.powerShellDefaultVersion.toLowerCase()
449-
|| settings.developer.editorServicesLogLevel.toLowerCase() !== this.sessionSettings.developer.editorServicesLogLevel.toLowerCase()
450-
|| settings.developer.bundledModulesPath.toLowerCase() !== this.sessionSettings.developer.bundledModulesPath.toLowerCase()
447+
(settings.cwd !== this.sessionSettings.cwd
448+
|| settings.powerShellDefaultVersion !== this.sessionSettings.powerShellDefaultVersion
449+
|| settings.developer.editorServicesLogLevel !== this.sessionSettings.developer.editorServicesLogLevel
450+
|| settings.developer.bundledModulesPath !== this.sessionSettings.developer.bundledModulesPath
451451
|| settings.developer.editorServicesWaitForDebugger !== this.sessionSettings.developer.editorServicesWaitForDebugger
452452
|| settings.integratedConsole.useLegacyReadLine !== this.sessionSettings.integratedConsole.useLegacyReadLine
453453
|| settings.integratedConsole.startInBackground !== this.sessionSettings.integratedConsole.startInBackground
@@ -872,7 +872,8 @@ Type 'help' to get help.
872872
prompt: "Open an Issue",
873873
action: async (): Promise<void> => {
874874
await vscode.commands.executeCommand("PowerShell.GenerateBugReport");
875-
}}]
875+
}
876+
}]
876877
);
877878
}
878879

@@ -883,7 +884,8 @@ Type 'help' to get help.
883884
action: async (): Promise<void> => {
884885
await vscode.env.openExternal(
885886
vscode.Uri.parse("https://aka.ms/get-powershell-vscode"));
886-
}}]
887+
}
888+
}]
887889
);
888890
}
889891

@@ -894,7 +896,8 @@ Type 'help' to get help.
894896
action: async (): Promise<void> => {
895897
await vscode.env.openExternal(
896898
vscode.Uri.parse("https://dotnet.microsoft.com/en-us/download/dotnet-framework"));
897-
}}]
899+
}
900+
}]
898901
);
899902
}
900903

0 commit comments

Comments
 (0)