Web lists-archives.com

Re: [PATCH] git-prompt: fix reading files with windows line endings




Hi Johannes,

On 29 Nov 2017 15:27, Johannes Schindelin wrote:
>> diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
>> index c6cbef38c2..71a64e7959 100644
>> --- a/contrib/completion/git-prompt.sh
>> +++ b/contrib/completion/git-prompt.sh
>> @@ -282,7 +282,7 @@ __git_eread ()
>>  {
>>  	local f="$1"
>>  	shift
>> -	test -r "$f" && read "$@" <"$f"
>> +	test -r "$f" && read "$@" <"$f" && export $@="${!@%$'\r'}"
> 
> As far as I understand, $'\r' is a Bash-only construct, and this file
> (git-prompt.sh) is targeting other Unix shells, too.

Sorry, I wasn't really aware about this bash-ism. I agree that a generic
solution would be best.

> So how about using `tr -d '\r' <"$f" | read "$@"` instead?

That doesn't work for me. Apparently, the variable is always reset to ""
and hence the prompt will always display the shortened sha1.
Maybe it has something to do with variable scoping inside the backtick
evaluation?

> Or maybe keep with the Bash construct, but guarded behind a test that we
> area actually running in Bash? Something like
> 
> 	test -z "$BASH" || IFS=$' \t\r\n'

Actually, this got me thinking and reading the POSIX.1-2008, specifically
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/read.html.

It seems POSIX states that IFS should be supported by read. This means
that it should be okay to just do

> test -r "$f" && IFS=" \t\r\n" read "$@" < "$f"

This would also get rid of the export and avoid introducing backtick
evaluation.

Regards,

Robert