From 8a496dc2697174b4f5395990131cd478aa68c949 Mon Sep 17 00:00:00 2001 From: mivirl Date: Mon, 4 Aug 2025 22:03:17 +0000 Subject: [PATCH] feat: Add shell-style quoting and escaping to edit and run commands Allows specifying a custom editor command with quoted spaces `${CRATE_ROOT}` and `${GPR_FILE}` are still replaced directly, not using parameter expansion. The argument to `alr run -a` has been changed to use the same quoting rules as `alr edit` instead of using the OS-specific rules provided by `GNAT.OS_Lib.Argument_String_To_List`. Quoting and token recognition uses the behavior for the shell command language from the POSIX.1-2024 standard with a few changes: - [2.2.2 Single-Quotes](https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_02_02) - No changes - [2.2.3 Double-Quotes](https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_02_02) - The dollar-sign is not used for parameter expansion, command substitution, or arithmetic expansion - The backquote is not used for command substitution - The backslash is an escape character within double quotes only when immediately followed by `\` or `"` - [2.3 Token Recognition](https://pubs.opengroup.org/onlinepubs/9799919799/utilities/V3_chap02.html#tag_19_03) - Here-documents are not supported - Parameter expansion, command substitution, and arithmetic expansion are not supported - Comments are not supported - Alias substitution is not supported --- BREAKING.md | 1 + doc/user-changes.md | 19 ++ src/alire/alire-os_lib-subprocess.adb | 183 ++++++++++++++++++ src/alire/alire-os_lib-subprocess.ads | 14 ++ src/alire/alire-spawn.adb | 3 +- src/alire/alire-spawn.ads | 4 +- src/alr/alr-commands-run.adb | 1 - src/alr/alr-os_lib.adb | 24 +-- .../src/alr_tests-split_arguments.adb | 92 +++++++++ 9 files changed, 320 insertions(+), 21 deletions(-) create mode 100644 testsuite/tests_ada/src/alr_tests-split_arguments.adb diff --git a/BREAKING.md b/BREAKING.md index f3af3249..c5a212a0 100644 --- a/BREAKING.md +++ b/BREAKING.md @@ -11,6 +11,7 @@ may change. - manifest: array of licenses is no longer supported (SPDX expressions allow multiple licenses). - alr: setting a builtin without `--builtin` will emit a new warning. - alr: style checks are now disabled by default in all build profiles. +- alr: quoting follows shell rules for `editor.cmd` and `alr run -a` ### We are here diff --git a/doc/user-changes.md b/doc/user-changes.md index ec90ca99..1aa65dc5 100644 --- a/doc/user-changes.md +++ b/doc/user-changes.md @@ -6,6 +6,25 @@ stay on top of `alr` new features. ## Release `3.0` +### Support for quoting in custom editors, quoting changes for `alr run` + +PR [#XXXX](https://github.com/alire-project/alire/pull/XXXX) + +The `alr edit --set-editor` command now supports double quotes, single quotes, +and backslash escaping within `editor.cmd` using shell quoting rules. + +```shell +$ alr config --set --global editor.cmd "command with 'quoted arguments'" +``` + +`${CRATE_ROOT}` and `${GPR_FILE}` are still replaced directly + +Arguments passed using `alr run -a arguments` now use the same quoting format. + +```shell +$ alr run -a "list of 'quoted arguments'" +``` + ### New `--github` switch for `alr init` command PR [#1972](https://github.com/alire-project/alire/pull/1972) diff --git a/src/alire/alire-os_lib-subprocess.adb b/src/alire/alire-os_lib-subprocess.adb index 41f8ef52..72d9e057 100644 --- a/src/alire/alire-os_lib-subprocess.adb +++ b/src/alire/alire-os_lib-subprocess.adb @@ -1,3 +1,5 @@ +with Ada.Characters.Latin_1; +with Ada.Strings.Maps; with Ada.Text_IO; with Alire.Directories; @@ -93,6 +95,156 @@ package body Alire.OS_Lib.Subprocess is end if; end Locate_In_Path; + --------------------- + -- Split_Arguments -- + --------------------- + + function Split_Arguments (Arguments : String) return AAA.Strings.Vector is + Is_Escaped : Boolean := False; + In_Single_Quotes : Boolean := False; + In_Double_Quotes : Boolean := False; + In_Word : Boolean := False; + + Token : String (Arguments'Range); + Token_Last : Natural := 0; + Token_Vector : AAA.Strings.Vector := AAA.Strings.Empty_Vector; + + function Is_Blank (Char : Character) return Boolean is + Blank_Set : constant Ada.Strings.Maps.Character_Set + := Ada.Strings.Maps.To_Set + (" " & Ada.Characters.Latin_1.HT); + -- The character class in POSIX locale contains , + -- per chapter 3 (V1) section 3.45 of: + -- https://pubs.opengroup.org/onlinepubs/9799919799/basedefs/ + begin + return Ada.Strings.Maps.Is_In (Char, Blank_Set); + end Is_Blank; + + procedure Token_Append (Char : Character) is + begin + if not In_Word then + Token_Last := Token'First; + In_Word := True; + else + Token_Last := Token_Last + 1; + end if; + Token (Token_Last) := Char; + end Token_Append; + + procedure Token_Finish is + begin + if In_Word then + declare + Final_Token : constant String + := Token (Token'First .. Token_Last); + begin + Token_Vector.Append (Final_Token); + In_Word := False; + end; + end if; + end Token_Finish; + begin + -- Token recognition rules according to the POSIX.1-2024 standard + -- See chapter 2 (V3) section 2.3 of: + -- https://pubs.opengroup.org/onlinepubs/9799919799/utilities/ + for I in Arguments'Range loop + -- 1. This rule is handled after the loop + -- 2. This rule is skipped since operators are not supported + -- 3. This rule is skipped since operators are not supported + + -- 4. Handle escaping and quoting + if In_Single_Quotes then + if ''' = Arguments (I) then + In_Single_Quotes := False; + else + Token_Append (Arguments (I)); + end if; + goto Token_Loop_Continue; + + elsif In_Double_Quotes then + if Is_Escaped then + case Arguments (I) is + when '"' | '\' => + Token_Append (Arguments (I)); + when others => + Token_Append ('\'); + Token_Append (Arguments (I)); + end case; + Is_Escaped := False; + else + case Arguments (I) is + when '"' => + In_Double_Quotes := False; + when '\' => + Is_Escaped := True; + when others => + Token_Append (Arguments (I)); + end case; + end if; + goto Token_Loop_Continue; + + else + if Is_Escaped then + Token_Append (Arguments (I)); + Is_Escaped := False; + goto Token_Loop_Continue; + else + case Arguments (I) is + when ''' => + In_Single_Quotes := True; + goto Token_Loop_Continue; + when '"' => + In_Double_Quotes := True; + goto Token_Loop_Continue; + when '\' => + Is_Escaped := True; + goto Token_Loop_Continue; + when others => + null; + end case; + end if; + end if; + + -- 5. This rule is skipped since parameter expansion, command + -- substitution, and arithmetic expansion are not supported + -- 6. This rule is skipped since operators are not supported + + -- 7. End token on unquoted blank characters + if Is_Blank (Arguments (I)) then + Token_Finish; + goto Token_Loop_Continue; + end if; + + -- 8. Append current character to current token if part of a word + if In_Word then + Token_Append (Arguments (I)); + goto Token_Loop_Continue; + end if; + + -- 9. This rule is skipped since comments are not supported + + -- 10. Start new word + Token_Append (Arguments (I)); + + <> + end loop; + -- 1. Delimit current token on end of input + Token_Finish; + + if Is_Escaped then + Raise_Checked_Error + ("Unterminated escape sequence in command: " & Arguments); + elsif In_Single_Quotes then + Raise_Checked_Error + ("Unterminated single quote sequence in command: " & Arguments); + elsif In_Double_Quotes then + Raise_Checked_Error + ("Unterminated double quote sequence in command: " & Arguments); + end if; + + return Token_Vector; + end Split_Arguments; + ------------------- -- Checked_Spawn -- ------------------- @@ -175,6 +327,37 @@ package body Alire.OS_Lib.Subprocess is Dim_Output : Boolean := True) return Integer is (Spawn (Command, Arguments, Understands_Verbose, Dim_Output)); + --------------- + -- Spawn_Raw -- + --------------- + + procedure Spawn_Raw (Command : String; + Arguments : AAA.Strings.Vector) + is + Code : Integer; + begin + + declare + Full_Path : constant String := + Alire.OS_Lib.Subprocess.Locate_In_Path (Command); + Parsed_Arguments : constant GNAT.OS_Lib.Argument_List_Access + := To_Argument_List (Arguments); + begin + if Full_Path = "" then + Alire.Raise_Checked_Error + ("Executable not found in PATH when spawning: " + -- & TTY.Terminal (Command & " " & Arguments)); + & TTY.Terminal (Command)); + end if; + + Code := GNAT.OS_Lib.Spawn (Full_Path, Parsed_Arguments.all); + end; + + if Code /= 0 then + raise Child_Failed with "Exit code:" & Code'Image; + end if; + end Spawn_Raw; + ----------- -- Spawn -- ----------- diff --git a/src/alire/alire-os_lib-subprocess.ads b/src/alire/alire-os_lib-subprocess.ads index 53604162..f344081f 100644 --- a/src/alire/alire-os_lib-subprocess.ads +++ b/src/alire/alire-os_lib-subprocess.ads @@ -7,6 +7,12 @@ package Alire.OS_Lib.Subprocess is function Locate_In_Path (Name : String) return String; -- Returns full path to Name command or "" if not found + function Split_Arguments (Arguments : String) return AAA.Strings.Vector; + -- Splits a string into arguments using shell-style quoting rules + -- (space/tab-separated words, double quotes to group space-separated + -- words, backslash `\` to escape quotes, and single quotes to group + -- without escaping between the quotes) + procedure Checked_Spawn (Command : String; Arguments : AAA.Strings.Vector; @@ -43,4 +49,12 @@ package Alire.OS_Lib.Subprocess is Dim_Output : Boolean := True) return Integer; -- Doesn't capture output but doesn't fail on error either + Child_Failed : exception; + procedure Spawn_Raw + (Command : String; + Arguments : AAA.Strings.Vector); + -- Direct launch, without any shenanigangs on output, for example for + -- respawning the canonical version. + -- Raises CHILD_FAILED if exit code /= 0. + end Alire.OS_Lib.Subprocess; diff --git a/src/alire/alire-spawn.adb b/src/alire/alire-spawn.adb index 778c54aa..e0536fe4 100644 --- a/src/alire/alire-spawn.adb +++ b/src/alire/alire-spawn.adb @@ -36,7 +36,8 @@ package body Alire.Spawn is Replacements : Alire.Formatting.Replacements; Exec_Check : access procedure (Exec : String) := null) is - Args : AAA.Strings.Vector := AAA.Strings.Split (Cmd, ' '); + Args : AAA.Strings.Vector + := Alire.OS_Lib.Subprocess.Split_Arguments (Cmd); Exec : constant String := Args.First_Element; Replaced_Args : AAA.Strings.Vector; begin diff --git a/src/alire/alire-spawn.ads b/src/alire/alire-spawn.ads index 947bdff6..a5fbd22d 100644 --- a/src/alire/alire-spawn.ads +++ b/src/alire/alire-spawn.ads @@ -22,8 +22,8 @@ package Alire.Spawn is -- Launches a command from a string, according to the conventions used by -- commands configurable with `alr settings`. -- - -- Parses a space-separated string and performs ${} pattern replacements - -- (on arguments only). + -- Parses a shell-style quoted string and performs ${} pattern + -- replacements (on arguments only). -- -- Exec_Check is called with the executable name between parsing Cmd into -- arguments and executing the command, e.g. to check that the executable diff --git a/src/alr/alr-commands-run.adb b/src/alr/alr-commands-run.adb index f18669ae..34277ce6 100644 --- a/src/alr/alr-commands-run.adb +++ b/src/alr/alr-commands-run.adb @@ -5,7 +5,6 @@ with Alire.Platforms.Current; with Alr.Commands.Build; with Alr.Files; -with Alr.OS_Lib; with Alire.Utils; with GNAT.OS_Lib; diff --git a/src/alr/alr-os_lib.adb b/src/alr/alr-os_lib.adb index 5a6dc260..22107b9a 100644 --- a/src/alr/alr-os_lib.adb +++ b/src/alr/alr-os_lib.adb @@ -1,3 +1,4 @@ +with Ada.Exceptions; with Alire.OS_Lib.Subprocess; package body Alr.OS_Lib is @@ -48,28 +49,17 @@ package body Alr.OS_Lib is procedure Spawn_Raw (Command : String; Arguments : String := "") is - Code : Integer; begin Trace.Debug ("Spawning " & Command & " " & Arguments); - declare - Full_Path : constant String := - Alire.OS_Lib.Subprocess.Locate_In_Path (Command); begin - if Full_Path = "" then - Alire.Raise_Checked_Error - ("Executable not found in PATH when spawning: " - & TTY.Terminal (Command & " " & Arguments)); - end if; - - Code := GNAT.OS_Lib.Spawn - (Full_Path, - GNAT.OS_Lib.Argument_String_To_List (Arguments).all); + Alire.OS_Lib.Subprocess.Spawn_Raw + (Command, + Alire.OS_Lib.Subprocess.Split_Arguments (Arguments)); + exception + when E : Alire.OS_Lib.Subprocess.Child_Failed => + raise Child_Failed with Ada.Exceptions.Exception_Message (E); end; - - if Code /= 0 then - raise Child_Failed with "Exit code:" & Code'Image; - end if; end Spawn_Raw; end Alr.OS_Lib; diff --git a/testsuite/tests_ada/src/alr_tests-split_arguments.adb b/testsuite/tests_ada/src/alr_tests-split_arguments.adb new file mode 100644 index 00000000..cfba8532 --- /dev/null +++ b/testsuite/tests_ada/src/alr_tests-split_arguments.adb @@ -0,0 +1,92 @@ +with AAA.Strings; +with Alire.OS_Lib.Subprocess; + +procedure Alr_Tests.Split_Arguments is + + --------------------------- + -- Check_Split_Arguments -- + --------------------------- + + procedure Check_Split_Arguments is + use AAA.Strings; + use Alire.OS_Lib.Subprocess; + E : AAA.Strings.Vector renames AAA.Strings.Empty_Vector; + begin + -- Split on spaces + pragma Assert (Split_Arguments ("code example with spaces") = + E & "code" & "example" & "with" & "spaces"); + + -- Escaped spaces + pragma Assert (Split_Arguments ("code\ example\ with\ spaces") = + E & "code example with spaces"); + + -- Unnecessary escapes + pragma Assert (Split_Arguments ("\c\o\d\e \example \with \s\p\a\c\e\s") = + E & "code" & "example" & "with" & "spaces"); + + -- Double quotes + pragma Assert (Split_Arguments ("code ""example with"" spaces") = + E & "code" & "example with" & "spaces"); + + -- Single quotes + pragma Assert (Split_Arguments ("code 'example with' spaces") = + E & "code" & "example with" & "spaces"); + + -- Escaped double quotes + pragma Assert (Split_Arguments ("code \""example with\"" spaces") = + E & "code" & """example" & "with""" & "spaces"); + + -- Escaped single quotes + pragma Assert (Split_Arguments ("code \'example with\' spaces") = + E & "code" & "'example" & "with'" & "spaces"); + + -- Nested escaped double quotes + pragma Assert (Split_Arguments ("code ""example \"" with"" spaces") = + E & "code" & "example "" with" & "spaces"); + + -- Escaped closing single quote (closing quote cannot be escaped) + pragma Assert (Split_Arguments ("code 'example \' with spaces") = + E & "code" & "example \" & "with" & "spaces"); + + -- Nested double & single quotes + pragma Assert (Split_Arguments ("code ""example 'with spaces'""") = + E & "code" & "example 'with spaces'"); + + -- Unterminated escape should raise an exception + declare + Ignored : AAA.Strings.Vector; + begin + Ignored := Split_Arguments ("code example with spaces\"); + raise Program_Error with "Previous call should have raised"; + exception + when Checked_Error => + null; + end; + + -- Unterminated single quote should raise an exception + declare + Ignored : AAA.Strings.Vector; + begin + Ignored := Split_Arguments ("code example with 'spaces"); + raise Program_Error with "Previous call should have raised"; + exception + when Checked_Error => + null; + end; + + -- Unterminated double quote should raise an exception + declare + Ignored : AAA.Strings.Vector; + begin + Ignored := Split_Arguments ("code example with ""spaces"); + raise Program_Error with "Previous call should have raised"; + exception + when Checked_Error => + null; + end; + + end Check_Split_Arguments; + +begin + Check_Split_Arguments; +end Alr_Tests.Split_Arguments; -- 2.39.5