On 16/06/17 14:49, Johannes Schindelin wrote:
> Hi Junio,
> 
> On Thu, 15 Jun 2017, Junio C Hamano wrote:
> 
>> diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh
>> index 325ec75353..801bce25da 100755
>> --- a/t/t3420-rebase-autostash.sh
>> +++ b/t/t3420-rebase-autostash.sh
>> @@ -45,7 +45,7 @@ create_expected_success_am() {
>>  }
>>  
>>  create_expected_success_interactive() {
>> -    cr=$'\r' &&
>> +    cr=$(echo . | tr '.' '\015') &&
>>      cat >expected <<-EOF
>>      $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual)
>>      HEAD is now at $(git rev-parse --short feature-branch) third commit
> 
> This is still incorrect, as the \r\n (which $(echo . | tr \.\ '\015')
> would emit) is interpreted correctly as a line break on Windows, meaning
> that cr is now *empty*. Not what we want.
> 
> What I did is to replace the `cat` by `q_to_cr` (we have that lovely
> function, might just as well use it), replace `${cr}` by `Q` and skip the
> cr variable altogether.
> 
> Additionally, there is another huge problem: the "Applied autostash." is
> printed to stdout, not stderr, in line with how things were done in the
> shell version of rebase -i.
> 
> While this was just a minor bug previously, now we exercise that bug, by
> redirecting stderr to stdout and redirecting stdout to the file `actual`.
> Nothing says that stderr should be printed to that file before stdout, but
> that is exactly what the test case tries to verify.
> 
> There is only one slight problem: in my Git for Windows SDK, stdout is
> printed first.
> 
> The quick fix would be to redirect stderr and stdout independently.
> 
> However, I think it is time for that bug to be fixed: autostash messages
> should really go to stderr, just like the rest of them rebase messages.
> 
> I attached the patch, together with the two fixups to Phillip's patches,
> and a fixup for the autostash-messages-to-stderr patch that I think should
> be squashed in but I really ran out of time testing this.
> 
> Phillip, would you mind picking those changes up as you deem appropriate?

Will do, thanks for the patches

Reply via email to