#297: CustomShell: When parsing shell command wrapper, do not parse the command content with it.

Instead, join them together later.

This avoids parsing shell command content twice.

Author: Taitava

src/ShellCommandExecutor.ts

@@ -152,10 +152,19 @@ export class ShellCommandExecutor {
                 if (await parsing_process.processRest()) {
                     // Parsing the rest of the variables succeeded
                     // Execute the shell command.
+                    // TODO: Create a new class ShellCommandParsingProcess (extends ParsingProcess) whose .getParsingResults() returns the shell_command_parsing_result below. I.e. extract the parsing result conversion logic to a separate class. Note that the class should only accept shell_command_parsing_map (defined in TShellCommand.ts) as it's original_contents parameter/property.
                     const parsing_results = parsing_process.getParsingResults();
+                    const unwrappedShellCommandContent: string = (parsing_results.shellCommandContent as ParsingResult).parsed_content as string;
                     const shell_command_parsing_result: ShellCommandParsingResult = {
-                        unwrappedShellCommandContent: (parsing_results.unwrappedShellCommandContent as ParsingResult).parsed_content as string,
-                        wrappedShellCommandContent: (parsing_results.wrappedShellCommandContent as ParsingResult).parsed_content as string,
+                        unwrappedShellCommandContent: unwrappedShellCommandContent,
+                        wrappedShellCommandContent: parsing_results.shellCommandWrapper?.parsed_content
+                            ? TShellCommand.wrapShellCommandContent(
+                                this.plugin,
+                                unwrappedShellCommandContent,
+                                parsing_results.shellCommandWrapper.parsed_content as string,
+                            )
+                            : unwrappedShellCommandContent // No wrapper, use unwrapped shell command content as wrapped.
+                        ,
                         alias: (parsing_results["alias"] as ParsingResult).parsed_content as string,
                         environment_variable_path_augmentation: (parsing_results.environment_variable_path_augmentation as ParsingResult).parsed_content as string,
                         stdinContent: parsing_results.stdinContent?.parsed_content as string,

src/TShellCommand.ts

@@ -30,7 +30,10 @@ import {getSC_Events} from "./events/SC_EventList";
 import {debugLog} from "./Debug";
 import {Command} from "obsidian";
 import {VariableSet} from "./variables/loadVariables";
-import {getUsedVariables} from "./variables/parseVariables";
+import {
+    getUsedVariables,
+    parseVariableSynchronously,
+} from "./variables/parseVariables";
 import {
     createPreaction,
     CustomVariable,
@@ -53,6 +56,7 @@ import {OutputStream} from "./output_channels/OutputChannelCode";
 import {OutputWrapper} from "./models/output_wrapper/OutputWrapper";
 import {Shell} from "./shells/Shell";
 import {getShell} from "./shells/ShellFunctions";
+import {Variable_ShellCommandContent} from "./variables/Variable_ShellCommandContent";
 
 export interface TShellCommandContainer {
     [key: string]: TShellCommand,
@@ -124,7 +128,7 @@ export class TShellCommand {
      *
      * Does not include any possible Shell provided augmentations to the shell command content.
      */
-    public getUnwrappedShellCommandContent(): string {
+    public getShellCommandContent(): string {
         // Check if the shell command has defined a specific command for this operating system.
         const platformSpecificShellCommand: string | undefined = this.configuration.platform_specific_commands[getOperatingSystem()];
         if (undefined === platformSpecificShellCommand) {
@@ -137,18 +141,6 @@ export class TShellCommand {
         }
     }
 
-    /**
-     * Returns a shell command string specific for the current operating system, or a generic shell command if this shell
-     * command does not have an explicit version for the current OS.
-     *
-     * Includes possible Shell provided augmentations to the shell command content.
-     *
-     * @private Can be made public if needed.
-     */
-    private getWrappedShellCommandContent(): string {
-        return this.getShell().wrapShellCommandContent(this.getUnwrappedShellCommandContent());
-    }
-
     /**
      * Returns a version of the shell command that should be used if no platform specific command is defined for the
      * current platform. If you plan to use this for execution, consider using getShellCommand() instead, as it takes the
@@ -202,7 +194,7 @@ export class TShellCommand {
      * TODO: Use this method in all places where similar logic is needed. I guess generateObsidianCommandName() is the only place left.
      */
     public getAliasOrShellCommand(): string {
-        return this.configuration.alias || this.getUnwrappedShellCommandContent(); // TODO: Use this.getAlias().
+        return this.configuration.alias || this.getShellCommandContent(); // TODO: Use this.getAlias().
     }
 
     public getConfirmExecution() {
@@ -442,8 +434,8 @@ export class TShellCommand {
         return new ParsingProcess<shell_command_parsing_map>(
             this.plugin,
             {
-                unwrappedShellCommandContent: this.getUnwrappedShellCommandContent(),
-                wrappedShellCommandContent: this.getWrappedShellCommandContent(),
+                shellCommandContent: this.getShellCommandContent(),
+                shellCommandWrapper: this.getShell().getShellCommandWrapper(),
                 alias: this.getAlias(),
                 environment_variable_path_augmentation: getPATHAugmentation(this.plugin) ?? "",
                 stdinContent: this.configuration.input_contents.stdin ?? undefined,
@@ -567,7 +559,16 @@ export class TShellCommand {
 
         // Get a list CustomVariables that the shell command uses.
         const custom_variables = new VariableSet();
-        const readVariablesFrom = this.getWrappedShellCommandContent(); // FIXME: Should actually include also other stuff that uses variables when executing shell commands, e.g. output wrappers. I think the best solution would be to call TShellCommand.createParsingProcess() and then get all variables from all the parseable content. Afterwards, delete the parsing process without actually parsing it, as the result would not be needed anyway. ParsingProcess class could have a new method named .getUsedVariables() that would wrap the global getUsedVariables() function and get variables used in that particular ParsingProcess.
+        const shellCommandWrapper: string | undefined = this.getShell().getShellCommandWrapper();
+        const readVariablesFrom = shellCommandWrapper
+            ? TShellCommand.wrapShellCommandContent(
+                this.plugin,
+                this.getShellCommandContent(),
+                shellCommandWrapper,
+            )
+            : this.getShellCommandContent() // Use unwrapped content.
+        ;
+        // FIXME: readVariablesFrom should actually include also other stuff that uses variables when executing shell commands, e.g. output wrappers. I think the best solution would be to call TShellCommand.createParsingProcess() and then get all variables from all the parseable content. Afterwards, delete the parsing process without actually parsing it, as the result would not be needed anyway. ParsingProcess class could have a new method named .getUsedVariables() that would wrap the global getUsedVariables() function and get variables used in that particular ParsingProcess.
         for (const custom_variable of getUsedVariables(this.plugin, readVariablesFrom)) {
             // Check that the variable IS a CustomVariable.
             if (custom_variable instanceof CustomVariable) { // TODO: Remove the check and pass only a list of CustomVariables to getUsedVariables().
@@ -613,6 +614,31 @@ export class TShellCommand {
         }
         return t_shell_commands[this_index - 1];
     }
+
+    /**
+     * Replaces all occurrences of {{shell_command_content}} in shellCommandWrapper with shellCommandContent.
+     *
+     * @param plugin
+     * @param shellCommandContent
+     * @param shellCommandWrapper
+     */
+    public static wrapShellCommandContent(plugin: SC_Plugin, shellCommandContent: string, shellCommandWrapper: string) {
+        const debugMessageBase = `${this.constructor.name}.wrapShellCommandContent(): `;
+        debugLog(`${debugMessageBase}Using wrapper: ${shellCommandWrapper} for shell command: ${shellCommandContent}`);
+
+        // Wrap the shell command.
+        const wrapperParsingResult = parseVariableSynchronously(
+            shellCommandWrapper,
+            new Variable_ShellCommandContent(plugin, shellCommandContent),
+        );
+        if (!wrapperParsingResult.succeeded) {
+            // {{shell_command_content}} is so simple that there should be no way for its parsing to fail.
+            throw new Error("{{shell_command_content}} parsing failed, although it should not fail.");
+        }
+
+        debugLog(`${debugMessageBase}Wrapped shell command: ${wrapperParsingResult.parsed_content}`);
+        return wrapperParsingResult.parsed_content as string; // It's always string at this point, as .succeeded is checked above.
+    }
 }
 
 export class TShellCommandMap extends Map<string, TShellCommand> {}
@@ -637,8 +663,8 @@ export interface ShellCommandParsingResult {
 export type ShellCommandParsingProcess = ParsingProcess<shell_command_parsing_map>;
 
 type shell_command_parsing_map = {
-    unwrappedShellCommandContent: string,
-    wrappedShellCommandContent: string,
+    shellCommandContent: string,
+    shellCommandWrapper?: string,
     alias: string,
     environment_variable_path_augmentation: string,
     stdinContent?: string,

src/events/SC_MenuEvent.ts

@@ -66,7 +66,7 @@ export abstract class SC_MenuEvent extends SC_WorkspaceEvent {
                         // Parsing succeeded.
                         const parsing_results = parsing_process.getParsingResults();
                         const aliasParsingResult: ParsingResult = parsing_results["alias"] as ParsingResult; // as ParsingResult: Tells TypeScript that the object exists.
-                        const unwrappedShellCommandParsingResult: ParsingResult = parsing_results.unwrappedShellCommandContent as ParsingResult; // as ParsingResult: Tells TypeScript that the object exists.
+                        const unwrappedShellCommandParsingResult: ParsingResult = parsing_results.shellCommandContent as ParsingResult; // as ParsingResult: Tells TypeScript that the object exists.
                         title = aliasParsingResult.parsed_content || unwrappedShellCommandParsingResult.parsed_content as string; // Try to use a parsed alias, but if no alias is available, use a (short, unwrapped) parsed shell command instead. as string = parsed shell command always exist when the parsing itself has succeeded.
                         debugLog(debugLogBaseMessage + "Menu title parsing succeeded. Will use title: " + title);
                         menuItem.setTitle(title);

src/main.ts

@@ -330,7 +330,10 @@ export default class SC_Plugin extends Plugin {
 				// Try to process variables that can be processed before performing preactions.
 				await parsing_process.process();
 			}
-			if (parsing_process.getParsingResults().wrappedShellCommandContent?.succeeded) { // .wrappedShellCommandContent should always be present (even if parsing did not succeed), but if it's not, show errors in the else block.
+            const parsingResults = parsing_process.getParsingResults();
+            const shellCommandContentParsingSucceeded = parsingResults.shellCommandContent?.succeeded; // .shellCommandContent should always be present (even if parsing did not succeed), but if it's not, show errors in the else block.
+            const shellCommandWrapperParsingSucceeded = parsingResults.shellCommandWrapper ? parsingResults.shellCommandWrapper.succeeded : true; // If no wrapper is present, pass.
+            if (shellCommandContentParsingSucceeded && shellCommandWrapperParsingSucceeded) { // FIXME: This should not rely on just one (or two) content's parsing result, it should check all of them. Use parsing_process.getErrorMessages().length === 0 to check all parsed content.
 				// The command was parsed correctly.
 				const executor_instance = new ShellCommandExecutor( // Named 'executor_instance' because 'executor' is another constant.
 					this,
@@ -348,7 +351,7 @@ export default class SC_Plugin extends Plugin {
 		// Register an Obsidian command
 		const obsidian_command: Command = {
 			id: this.generateObsidianCommandId(shell_command_id),
-			name: generateObsidianCommandName(this, t_shell_command.getUnwrappedShellCommandContent(), t_shell_command.getAlias()), // Will be overridden in command palette, but this will probably show up in hotkey settings panel - at least if command palette has not been opened yet since launching Obsidian. Also note that on some systems async variable parsing might make name generation take so long that after the name is updated in the Command object, it will not reflect in the visual menu anymore. This has happened at least on File menu on macOS, so I suspect it might concern Command palette, too. See GitHub #313 / #314 for more information. As this early name setting has been in place from the very beginning of the SC plugin, it (according to my knowledge) has protected the command palette from having similar problems that context menus have had.
+			name: generateObsidianCommandName(this, t_shell_command.getShellCommandContent(), t_shell_command.getAlias()), // Will be overridden in command palette, but this will probably show up in hotkey settings panel - at least if command palette has not been opened yet since launching Obsidian. Also note that on some systems async variable parsing might make name generation take so long that after the name is updated in the Command object, it will not reflect in the visual menu anymore. This has happened at least on File menu on macOS, so I suspect it might concern Command palette, too. See GitHub #313 / #314 for more information. As this early name setting has been in place from the very beginning of the SC plugin, it (according to my knowledge) has protected the command palette from having similar problems that context menus have had.
 			// Use 'checkCallback' instead of normal 'callback' because we also want to get called when the command palette is opened.
 			checkCallback: (is_opening_command_palette): boolean | void => { // If is_opening_command_palette is true, then the return type is boolean, otherwise void.
 				if (is_opening_command_palette) {
@@ -373,7 +376,7 @@ export default class SC_Plugin extends Plugin {
                                 // Rename Obsidian command
                                 const parsingResults = parsing_process.getParsingResults();
                                 /** Don't confuse this name with ShellCommandParsingResult interface! The properties are very different. TODO: Rename ShellCommandParsingResult to something else. */
-                                const shellCommandParsingResult: ParsingResult = parsingResults.unwrappedShellCommandContent as ParsingResult; // Use 'as' to denote that properties exist on this line and below.
+                                const shellCommandParsingResult: ParsingResult = parsingResults.shellCommandContent as ParsingResult; // Use 'as' to denote that properties exist on this line and below.
                                 const aliasParsingResult: ParsingResult = parsingResults["alias"] as ParsingResult;
                                 const parsedShellCommand: string = shellCommandParsingResult.parsed_content as string;
                                 const parsedAlias: string = aliasParsingResult.parsed_content as string;
@@ -383,13 +386,13 @@ export default class SC_Plugin extends Plugin {
                                 this.cached_parsing_processes[t_shell_command.getId()] = parsing_process;
                             } else {
                                 // Parsing failed, so use unparsed t_shell_command.getShellCommand() and t_shell_command.getAlias().
-                                t_shell_command.renameObsidianCommand(t_shell_command.getUnwrappedShellCommandContent(), t_shell_command.getAlias());
+                                t_shell_command.renameObsidianCommand(t_shell_command.getShellCommandContent(), t_shell_command.getAlias());
                                 this.cached_parsing_processes[t_shell_command.getId()] = undefined;
                             }
                         });
                     } else {
                         // Parsing is disabled, so use unparsed t_shell_command.getShellCommand() and t_shell_command.getAlias().
-                        t_shell_command.renameObsidianCommand(t_shell_command.getUnwrappedShellCommandContent(), t_shell_command.getAlias());
+                        t_shell_command.renameObsidianCommand(t_shell_command.getShellCommandContent(), t_shell_command.getAlias());
                         this.cached_parsing_processes[t_shell_command.getId()] = undefined;
                     }
 

src/models/custom_shell/CustomShellSettingsModal.ts

@@ -58,6 +58,7 @@ import {SettingFieldGroup} from "../../settings/SC_MainSettingsTab";
 import * as path from "path";
 import * as fs from "fs";
 import {getPATHEnvironmentVariableName} from "../../settings/setting_elements/PathEnvironmentVariableFunctions";
+import {TShellCommand} from "../../TShellCommand";
 
 export class CustomShellSettingsModal extends SC_Modal {
 
@@ -484,9 +485,17 @@ export class CustomShellSettingsModal extends SC_Modal {
                     this.plugin.newError("The test shell command is empty.");
                     return;
                 }
+                const wrappedShellCommandContent: string = customShellConfiguration.shell_command_wrapper
+                    ? TShellCommand.wrapShellCommandContent(
+                        this.plugin,
+                        customShellConfiguration.shell_command_test,
+                        customShellConfiguration.shell_command_wrapper,
+                    )
+                    : customShellConfiguration.shell_command_test // No wrapper, use unwrapped shell command content.
+                ;
                 const testShellCommandParsingResult: ParsingResult = await parseVariables(
                     this.plugin,
-                    this.getCustomShell().wrapShellCommandContent(customShellConfiguration.shell_command_test),
+                    wrappedShellCommandContent,
                     this.getCustomShell(),
                     true, // Enable escaping, but if this.customShellInstance.configuration.escaper is "none", then escaping is prevented anyway.
                     null, // No TShellCommand, so no access for default values.