Re: [PATCH] git stash: one bug and one feature request

[Junio C Hamano <gitster@pobox.com>]

"Marco Costalba" <mcostalba@gmail.com> writes:

> This low level run() should know nothing about the semantic of the
> command or the outputted data, but should detect command failing,
> because failing reporting framework is unified and is the same for
> each type of command.

That sounds like a framework generalized in a wrong way to me.

> Please note that also gitk uses the same approach, indeed from
> http://ftp.tcl.tk/man/tcl8.5/tutorial/Tcl26.html you can read:
> ...

Heh, as if tcl is a textbook of good programming style.

> I can also black list not commonly behaving programs, but in case of
> git-stash a fail to see why to choose a not standard behaviour when
> not needed.

I do not offhand see a reason it would _hurt_ for this
particular case (git-stash) to write the diagnostics we
currently spit out to stderr to stdout.  My objection is
primarily because I do not think "never writing to stderr if
there is no error" is standard behaviour AT ALL.

IOW, I do have much less objections to what your patch actually
does, than I have problems with the way the reason for the
change is stated.  The change is not fixing anything to conform
to some standard behaviour.  It is more about bending
(admittedly only slightly) backwards to help broken callers.
That is what I have most trouble with.