On Fri, May 8, 2015 at 2:44 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>> On Fri, May 8, 2015 at 1:49 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>> Thanks, this looks good. Will apply with a little bit of tweak in
>>> the log message.
>>
>> Hmm, I would say that the changes to debug() and say() should either
>> be dropped or moved to a separate patch (along with the first
>> paragraph of the commit message). With the introduction of the
>> progress() abstraction, there is no longer any need for changes to
>> say(), and the "better portability" rationale for changing say() and
>> debug() is never properly explained, and is thus nebulous at best.
>
> I justified them in this way.
>
> contrib/subtree: portability fix for string printing
>
> 'echo -n' is not portable, but this script used it as a way to give
> a string followed by a carriage return for progress messages.
> Introduce a new helper shell function "progress" and use printf as a
> more portable way to do this. As a side effect, this makes it
> unnecessary to have a raw CR in our source, which can be munged in
> some shells. For example, MsysGit trims CR before executing a shell
> script file in order to make it work right on Windows even if it
> uses CRLF as linefeeds.
Very nicely explained.
> While at it, replace "echo" using printf in debug() and say() to
> avoid tempting people introducing the same bug.
Okay, this works as reasonable justification for including those
changes in the same patch.
It might read a bit more fluidly if rephrased something like this:
While at it, replace 'echo' with 'printf' in debug() and say() to
eliminate the temptation of reintroducing the same bug.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html