#297: CustomShellSettingsModal: Show a warning if binary path does no…

…t seem to exist.

Also removes binary file existence check from CustomShell.augmentSpawn() because it gave false error messages for relative shell binary paths that are available via the PATH environment variable, but not detectable as files in the current working directory.

The new warning text system in the settings modal should be a good-enough safeguard against broken shell binary paths, and it's also shown to the user  earlier, not late as when executing something.

src/Common.ts

@@ -36,6 +36,8 @@ import SC_Plugin from "./main";
 import {shell} from "electron";
 // @ts-ignore Electron is installed.
 import {clipboard} from "electron";
+import * as fs from "fs";
+import * as process from "process";
 
 export function getVaultAbsolutePath(app: App) {
     // Original code was copied 2021-08-22 from https://github.com/phibr0/obsidian-open-with/blob/84f0e25ba8e8355ff83b22f4050adde4cc6763ea/main.ts#L66-L67
@@ -215,6 +217,27 @@ export function extractFileParentPath(file_path: string) {
     return path.parse(file_path).dir;
 }
 
+/**
+ * Same as fs.existsSync(binaryPath), but if on Windows, also looks up file names appended with extensions from the
+ * PATHEXT environment variable. This can notice that e.g. "CMD" exists as a file name, when it's checked as "CMD.EXE".
+ */
+export function binaryFilePathExists(binaryPath: string): boolean {
+    if (fs.existsSync(binaryPath)) {
+        return true;
+    }
+    if (isWindows()) {
+        // Windows: Binary path may be missing a file extension, but it's still a valid and working path, so check
+        // the path with additional extensions, too.
+        const pathExt = process.env.PATHEXT ?? "";
+        for (const extension of pathExt.split(";")) {
+            if (fs.existsSync(binaryPath + extension)) {
+                return true;
+            }
+        }
+    }
+    return false;
+}
+
 // eslint-disable-next-line @typescript-eslint/ban-types
 export function joinObjectProperties(object: {}, glue: string) {
     let result = "";

src/models/custom_shell/CustomShellSettingsModal.ts

@@ -30,6 +30,7 @@ import {
     PlatformNamesMap,
 } from "../../settings/SC_MainSettings";
 import {
+    binaryFilePathExists,
     createMultilineTextElement,
     getOperatingSystem,
 } from "../../Common";
@@ -52,6 +53,7 @@ import {Variable_FilePath} from "../../variables/Variable_FilePath";
 import {createAutocomplete} from "../../settings/setting_elements/Autocomplete";
 import {Variable_ShellCommandContent} from "../../variables/Variable_ShellCommandContent";
 import {SettingFieldGroup} from "../../settings/SC_MainSettingsTab";
+import * as path from "path";
 
 export class CustomShellSettingsModal extends SC_Modal {
 
@@ -193,9 +195,40 @@ export class CustomShellSettingsModal extends SC_Modal {
                 .onChange(async (newBinaryPath: string) => {
                     this.getCustomShellConfiguration().binary_path = newBinaryPath;
                     await this.plugin.saveSettings();
+                    updateBinaryPathWarning();
                 }),
             )
         ;
+        const binaryPathWarningDescription = new Setting(containerElement)
+            .setClass("SC-full-description")
+        ;
+        const updateBinaryPathWarning = () => {
+            const binaryPath = this.getCustomShellConfiguration().binary_path;
+            if (binaryFilePathExists(this.getCustomShellConfiguration().binary_path)) {
+                // OK: The binary path exists.
+                binaryPathWarningDescription.setDesc("Good, " + binaryPath + " exists.");
+                binaryPathWarningDescription.descEl.addClass("SC-text-right"); // Short text should be aligned to the right, so that they are under the binary path input field.
+            } else {
+                // The binary path does not exist.
+                if (this.getCustomShellConfiguration().host_platform === getOperatingSystem()) {
+                    // The shell is supposed to work on the current operating system.
+                    const notExistsText = `Note: ${binaryPath} does not seem to exist.`;
+                    const testShellText = " You can test the shell at the bottom of this modal.";
+                    if (path.isAbsolute(binaryPath)) {
+                        // The path is absolute, so it's probable that the shell won't work, as absolute binary paths are not likely to be found via the PATH environment variable.
+                        binaryPathWarningDescription.setDesc(`${notExistsText} It's good that the path is absolute, but maybe it contains a typing error?${testShellText}`);
+                    } else {
+                        // The path is relative, so it might be executable, if it happens to be findable via the PATH environment variable.
+                        binaryPathWarningDescription.setDesc(`${notExistsText} However, it might still work if the operating system recognises it as an executable command.${testShellText} If you encounter problems, try using an absolute path, i.e. a full path starting from the root of the file system.`);
+                    }
+                } else {
+                    // The shell is meant for another operating system, so it's understandable that the binary might not be present on this OS.
+                    binaryPathWarningDescription.setDesc("Note: The shell is configured to be used on a different operating system than " + PlatformNames[getOperatingSystem()] + ", so the existence of the binary file cannot be reliably verified.");
+                }
+                binaryPathWarningDescription.descEl.removeClass("SC-text-right"); // Long texts should be aligned to the left.
+            }
+        };
+        updateBinaryPathWarning();
     }
 
     private createShellArgumentsSetting(containerElement: HTMLElement): void {

src/shells/CustomShell.ts

@@ -32,7 +32,6 @@ import {
     normalizePath2,
 } from "../Common";
 import SC_Plugin from "../main";
-import * as fs from "fs";
 import {CustomShellConfiguration} from "../models/custom_shell/CustomShellModel";
 import {Variable_ShellCommandContent} from "../variables/Variable_ShellCommandContent";
 import {
@@ -155,12 +154,6 @@ export class CustomShell extends Shell {
 
     protected async augmentSpawn(spawnAugmentation: SpawnAugmentation, tShellCommand: TShellCommand | null, scEvent: SC_Event | null): Promise<boolean> {
         const debugLogBase = this.constructor.name + ".augmentSpawn(): ";
-        const shellBinaryPath = this.getBinaryPath();
-        if (!fs.existsSync(shellBinaryPath)) {
-            // Shell binary does not exist.
-            this.plugin.newError("Custom shell " + this.getName() + ": Binary path does not exist: " + shellBinaryPath);
-            return false; // Prevent execution.
-        }
 
         // Disable Node.js's builtin shell feature.
         // CustomShells need to be able to define their own shell invoking command line options, so the Node.js's shell feature would be too limited.
@@ -202,7 +195,7 @@ export class CustomShell extends Shell {
         spawnAugmentation.spawnArguments = parsedShellArguments;
 
         // Tell spawn() to use the shell binary path as an executable command.
-        spawnAugmentation.shellCommandContent = shellBinaryPath; // Needs to come AFTER the original shellCommandContent is taken to spawnArguments!
+        spawnAugmentation.shellCommandContent = this.getBinaryPath(); // Needs to come AFTER the original shellCommandContent is taken to spawnArguments!
         return true; // Allow execution.
     }
 

styles.css

@@ -52,6 +52,10 @@
     margin-left: 20px;
 }
 
+.SC-text-right {
+    text-align: right;
+}
+
 /*
  * SETTING GROUPS
  * Related setting fields combined together.