Re: [PATCH] contrib/subtree: remove contradicting use options on echo wrapper

[Paul Campbell <pcampbell@kemitix.net>]

On Fri, Feb 15, 2013 at 10:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Paul Campbell <pcampbell@kemitix.net> writes:
>
>> Remove redundant -n option and raw ^M in call to echo.
>>
>> Call to 'say' function, a wrapper of 'echo', passed the parameter -n, then
>> included a raw ^M newline in the end of the last parameter. Yet the -n option
>> is meant to suppress the addition of new line by echo.
>>
>> Signed-off-by: Paul Campbell <pcampbell@kemitix.net>
>
> I generally do not comment on comment on contrib/ material, and I am
> not familiar with subtree myself, but
>
>         for count in $(seq 0 $total)
>         do
>                 echo -n "$count/$total^M"
>                 ... do heavy lifting ...
>         done
>         echo "Done                  "
>
> is an idiomatic way to implement a progress meter without scrolling
> more important message you gave earlier to the user before entering
> the loop away.  The message appears, carrige-return moves the cursor
> to the beginning of the line without going to the next line, and the
> next iteration overwrites the previous count.  Finally, the progress
> meter is overwritten with the "Done" message.  Alternatively you can
> wrap it up with
>
>         echo
>         echo Done
>
> if you want to leave the final progress "100/100" before saying "Done."
>
> Isn't that what this piece of code trying to do?
>
>> ---
>>  contrib/subtree/git-subtree.sh | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
>> index 8a23f58..51146bd 100755
>> --- a/contrib/subtree/git-subtree.sh
>> +++ b/contrib/subtree/git-subtree.sh
>> @@ -592,7 +592,7 @@ cmd_split()
>>       eval "$grl" |
>>       while read rev parents; do
>>               revcount=$(($revcount + 1))
>> -             say -n "$revcount/$revmax ($createcount)
>> "
>> +             say "$revcount/$revmax ($createcount)"
>>               debug "Processing commit: $rev"
>>               exists=$(cache_get $rev)
>>               if [ -n "$exists" ]; then

[Apologies for resending this Junio. Forgot to hit reply all.]

Ah. I've not seen that done in shell before. In other languages I've
seen and used '\r'  for this purpose, rather than a raw ^M.

I was getting frustrated with it as my apparently braindead text
editor was converting it to a normal unix newline, which would then
keep getting picked up by git diff.

Please ignore my patch.

-- 
Paul [W] Campbell