Merge pull request #1813 from EliahKagan/prepare-doc

Revise and somewhat expand `Prepare` docs

gix-command/src/lib.rs

@@ -10,9 +10,10 @@ use std::{
 
 use bstr::{BString, ByteSlice};
 
-/// A structure to keep settings to use when invoking a command via [`spawn()`][Prepare::spawn()], after creating it with [`prepare()`].
+/// A structure to keep settings to use when invoking a command via [`spawn()`][Prepare::spawn()],
+/// after creating it with [`prepare()`].
 pub struct Prepare {
-    /// The command to invoke (either with or without shell depending on `use_shell`.
+    /// The command to invoke, either directly or with a shell depending on `use_shell`.
     pub command: OsString,
     /// Additional information to be passed to the spawned command.
     pub context: Option<Context>,
@@ -22,29 +23,29 @@ pub struct Prepare {
     pub stdout: std::process::Stdio,
     /// The way standard error is configured.
     pub stderr: std::process::Stdio,
-    /// The arguments to pass to the spawned process.
+    /// The arguments to pass to the process being spawned.
     pub args: Vec<OsString>,
-    /// environment variables to set in the spawned process.
+    /// Environment variables to set for the spawned process.
     pub env: Vec<(OsString, OsString)>,
     /// If `true`, we will use `shell_program` or `sh` to execute the `command`.
     pub use_shell: bool,
-    /// If `true`, `command` is assumed to be a command or path to the program to execute, and it will be shell-quoted
-    /// to assure it will be executed as is and without splitting across whitespace.
+    /// If `true`, `command` is assumed to be a command or path to the program to execute, and it
+    /// will be shell-quoted to assure it will be executed as is and without splitting across
+    /// whitespace.
     pub quote_command: bool,
     /// The name or path to the shell program to use instead of `sh`.
     pub shell_program: Option<OsString>,
-    /// If `true` (default `true` on windows and `false` everywhere else)
-    /// we will see if it's safe to manually invoke `command` after splitting
-    /// its arguments as a shell would do.
-    /// Note that outside of windows, it's generally not advisable as this
-    /// removes support for literal shell scripts with shell-builtins.
+    /// If `true` (default `true` on Windows and `false` everywhere else) we will see if it's safe
+    /// to manually invoke `command` after splitting its arguments as a shell would do.
     ///
-    /// This mimics the behaviour we see with `git` on windows, which also
-    /// won't invoke the shell there at all.
+    /// Note that outside of Windows, it's generally not advisable as this removes support for
+    /// literal shell scripts with shell-builtins.
     ///
-    /// Only effective if `use_shell` is `true` as well, as the shell will
-    /// be used as a fallback if it's not possible to split arguments as
-    /// the command-line contains 'scripting'.
+    /// This mimics the behaviour we see with `git` on Windows, which also won't invoke the shell
+    /// there at all.
+    ///
+    /// Only effective if `use_shell` is `true` as well, as the shell will be used as a fallback if
+    /// it's not possible to split arguments as the command-line contains 'scripting'.
     pub allow_manual_arg_splitting: bool,
 }
 
@@ -96,81 +97,119 @@ mod prepare {
 
     /// Builder
     impl Prepare {
-        /// If called, the command will be checked for characters that are typical for shell scripts, and if found will use `sh` to execute it
-        /// or whatever is set as [`with_shell_program()`](Self::with_shell_program()).
+        /// If called, the command will be checked for characters that are typical for shell
+        /// scripts, and if found will use `sh` to execute it or whatever is set as
+        /// [`with_shell_program()`](Self::with_shell_program()).
+        ///
         /// If the command isn't valid UTF-8, a shell will always be used.
         ///
-        /// If a shell is used, then arguments given here with [arg()](Self::arg) or [args()](Self::args) will be substituted via `"$@"` if it's not
-        /// already present in the command.
+        /// If a shell is used, then arguments given here with [arg()](Self::arg) or
+        /// [args()](Self::args) will be substituted via `"$@"` if it's not already present in the
+        /// command.
+        ///
         ///
-        /// If this method is not called, commands are always executed verbatim, without the use of a shell.
+        /// The [`command_may_be_shell_script_allow_manual_argument_splitting()`](Self::command_may_be_shell_script_allow_manual_argument_splitting())
+        /// and [`command_may_be_shell_script_disallow_manual_argument_splitting()`](Self::command_may_be_shell_script_disallow_manual_argument_splitting())
+        /// methods also call this method.
+        ///
+        /// If neither this method nor [`with_shell()`](Self::with_shell()) is called, commands are
+        /// always executed verbatim and directly, without the use of a shell.
         pub fn command_may_be_shell_script(mut self) -> Self {
             self.use_shell = self.command.to_str().map_or(true, |cmd| {
                 cmd.as_bytes().find_byteset(b"|&;<>()$`\\\"' \t\n*?[#~=%").is_some()
             });
             self
         }
 
-        /// If called, unconditionally use a shell to execute the command and its arguments, and `sh` to execute it,
-        /// or whatever is set as [`with_shell_program()`](Self::with_shell_program()).
+        /// If called, unconditionally use a shell to execute the command and its arguments.
+        ///
+        /// This uses `sh` to execute it, or whatever is set as
+        /// [`with_shell_program()`](Self::with_shell_program()).
         ///
-        /// If a shell is used, then arguments given here with [arg()](Self::arg) or [args()](Self::args) will be substituted via `"$@"` if it's not
-        /// already present in the command.
+        /// Arguments given here with [arg()](Self::arg) or [args()](Self::args) will be
+        /// substituted via `"$@"` if it's not already present in the command.
         ///
-        /// If this method is not called, commands are always executed verbatim, without the use of a shell.
+        /// If neither this method nor
+        /// [`command_may_be_shell_script()`](Self::command_may_be_shell_script()) is called,
+        /// commands are always executed verbatim and directly, without the use of a shell. (But
+        /// see [`command_may_be_shell_script()`](Self::command_may_be_shell_script()) on other
+        /// methods that call that method.)
         pub fn with_shell(mut self) -> Self {
             self.use_shell = true;
             self
         }
 
-        /// If  [`with_shell()`](Self::with_shell()) is set, then quote the command to assure its path is left intact.
+        /// Quote the command if it is run in a shell, so its path is left intact.
         ///
-        /// Note that this should not be used if the command is a script - quoting is only the right choice if it's known to be a program path.
+        /// This is only meaningful if the command has been arranged to run in a shell, either
+        /// unconditionally with [`with_shell()`](Self::with_shell()), or conditionally with
+        /// [`command_may_be_shell_script()`](Self::command_may_be_shell_script()).
+        ///
+        /// Note that this should not be used if the command is a script - quoting is only the
+        /// right choice if it's known to be a program path.
+        ///
+        /// Note also that this does not affect arguments passed with [arg()](Self::arg) or
+        /// [args()](Self::args), which do not have to be quoted by the *caller* because they are
+        /// passed as `"$@"` positional parameters (`"$1"`, `"$2"`, and so on).
         pub fn with_quoted_command(mut self) -> Self {
             self.quote_command = true;
             self
         }
 
-        /// Set the name or path to the shell `program` to use if a shell is to be used, to avoid using the default shell which is `sh`.
+        /// Set the name or path to the shell `program` to use if a shell is to be used, to avoid
+        /// using the default shell which is `sh`.
+        ///
+        /// Note that that shells that are not Bourne-style cannot be expected to work correctly,
+        /// because POSIX shell syntax is assumed when searching for and conditionally adding
+        /// `"$@"` to receive arguments, where applicable (and in the behaviour of
+        /// [`with_quoted_command()`](Self::with_quoted_command()), if called).
         pub fn with_shell_program(mut self, program: impl Into<OsString>) -> Self {
             self.shell_program = Some(program.into());
             self
         }
 
         /// Unconditionally turn off using the shell when spawning the command.
-        /// Note that not using the shell is the default so an effective use of this method
-        /// is some time after [`command_may_be_shell_script()`](Prepare::command_may_be_shell_script()) was called.
+        ///
+        /// Note that not using the shell is the default. So an effective use of this method
+        /// is some time after [`command_may_be_shell_script()`](Self::command_may_be_shell_script())
+        /// or [`with_shell()`](Self::with_shell()) was called.
         pub fn without_shell(mut self) -> Self {
             self.use_shell = false;
             self
         }
 
         /// Set additional `ctx` to be used when spawning the process.
         ///
-        /// Note that this is a must for most kind of commands that `git` usually spawns,
-        /// as at least they need to know the correct `git` repository to function.
+        /// Note that this is a must for most kind of commands that `git` usually spawns, as at
+        /// least they need to know the correct Git repository to function.
         pub fn with_context(mut self, ctx: Context) -> Self {
             self.context = Some(ctx);
             self
         }
 
-        /// Like [`command_may_be_shell_script()`](Prepare::command_may_be_shell_script()), but try to split arguments by hand if this can be safely done without a shell.
+        /// Like [`command_may_be_shell_script()`](Self::command_may_be_shell_script()), but try to
+        /// split arguments by hand if this can be safely done without a shell.
+        ///
+        /// This is useful on platforms where spawning processes is slow, or where many processes
+        /// have to be spawned in a row which should be sped up. Manual argument splitting is
+        /// enabled by default on Windows only.
         ///
-        /// This is useful on platforms where spawning processes is slow, or where many processes have to be spawned in a raw which should be sped up.
-        /// Manual argument splitting is enabled by default on Windows only.
+        /// Note that this does *not* check for the use of possible shell builtins. Commands may
+        /// fail or behave differently if they are available as shell builtins and no corresponding
+        /// external command exists, or the external command behaves differently.
         pub fn command_may_be_shell_script_allow_manual_argument_splitting(mut self) -> Self {
             self.allow_manual_arg_splitting = true;
             self.command_may_be_shell_script()
         }
 
-        /// Like [`command_may_be_shell_script()`](Prepare::command_may_be_shell_script()), but don't allow to bypass the shell even if manual argument splitting
-        /// can be performed safely.
+        /// Like [`command_may_be_shell_script()`](Self::command_may_be_shell_script()), but don't
+        /// allow to bypass the shell even if manual argument splitting can be performed safely.
         pub fn command_may_be_shell_script_disallow_manual_argument_splitting(mut self) -> Self {
             self.allow_manual_arg_splitting = false;
             self.command_may_be_shell_script()
         }
 
-        /// Configure the process to use `stdio` for _stdin.
+        /// Configure the process to use `stdio` for _stdin_.
         pub fn stdin(mut self, stdio: Stdio) -> Self {
             self.stdin = stdio;
             self
@@ -180,7 +219,7 @@ mod prepare {
             self.stdout = stdio;
             self
         }
-        /// Configure the process to use `stdio` for _stderr.
+        /// Configure the process to use `stdio` for _stderr_.
         pub fn stderr(mut self, stdio: Stdio) -> Self {
             self.stderr = stdio;
             self
@@ -256,7 +295,7 @@ mod prepare {
                                 prep.command.push(" \"$@\"");
                             } else {
                                 gix_trace::debug!(
-                                    "Will not add '$@' to '{:?}' as it seems to contain it already",
+                                    "Will not add '\"$@\"' to '{:?}' as it seems to contain '$@' already",
                                     prep.command
                                 );
                             }