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

From: Robert Abel <rabel@robertabel.eu>
Date: Nov 29, 2017, 22:09 UTC
Message-ID: <d57e4cb9-b0b4-314e-370a-e0db58a2a7da@robertabel.eu>
In-Reply-To: <alpine.DEB.2.21.1.1711291519290.6482@virtualbox>

Hi Johannes,

On 29 Nov 2017 15:27, Johannes Schindelin wrote:

Show 13 quoted lines

>> 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