Web lists-archives.com

Re: [PATCH v2 1/2] git-prompt: make __git_eread intended use explicit




Robert Abel <rabel@xxxxxxxxxxxxx> writes:

> __git_eread is used to read a single line of a given file (if it exists)
> into a variable without the EOL. All six current users of __git_eread
> use it that way and don't expect multi-line content.

Changing $@ to $2 does not change whether this is about "multi-line"
or not.  What you are changing is that the original was prepared to
be given two or more variable names, and split an input line at IFS
into multiple tokens to be assigned to these variables, but with
this change, the caller can only use one variable and this function
will not split the line and store it into that single variable.

The above can easily be fixed with a bit of rewording, perhaps like:

    ... that way.  We do not need to split the line into tokens and
    assign them to multiple variables---reading only into a single
    variable needs to be supported.

While reviewing this patch, I also wondered if the "read" wants to
become "read -r" or something that is even safer than simply
avoiding tokenization, but after scanning to see exactly which files
__git_eread is used to read from, I do not think it matters (the
input will not have a backslash that would want to be protected from
'read'), so this should be OK.

> Signed-off-by: Robert Abel <rabel@xxxxxxxxxxxxx>
> ---
>  contrib/completion/git-prompt.sh | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
> index c6cbef38c..41a471957 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -278,11 +278,12 @@ __git_ps1_colorize_gitstring ()
>  	r="$c_clear$r"
>  }
>  
> +# Helper function to read the first line of a file into a variable.
> +# __git_eread requires 2 arguments, the file path and the name of the
> +# variable, in that order.
>  __git_eread ()
>  {
> -	local f="$1"
> -	shift
> -	test -r "$f" && read "$@" <"$f"
> +	test -r "$1" && read "$2" <"$1"
>  }
>  
>  # __git_ps1 accepts 0 or 1 arguments (i.e., format string)