Hi Junio, On Wed, 27 Jul 2016, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > > > I am not yet sure if it makes sense to mix both the regular output > > and an error into the same buffer for the callers to process (your > > "reason 1)" above), and this looks like a wrong way to allow a > > caller that wants no output (your "reason 2)" above). A caller that > > wants to massage the output would want to know which ones are errors > > and which ones are not, I would imagine, and setting a knob to make > > both output() and err() a no-op would be a more suitable way to give > > a caller a total silence. > > I actually now see how this would work well for "reason 2)". If a > caller wants to run the function and wants to pretend as if it did > not run anything when it failed, for example, using this to spool > all output and error to a strbuf and discard it when the function > returns an error, and emit the spooled output to standard output and > standard error in the order the lines were collected when the > function returns a success, would be a good way to do so. That is actually the exact opposite of the intended usage: when any `pick` in an interactive rebase succeeds, its output is discarded so as not to bother the user. We show the complete output only when it fails. > That however brings me back to the "reason 1" thing. Shouldn't the > spooling be done not just with a single strbuf, but with an array of > <bool, const char *> tuples, where the bool says if it is for the > standard output, and the string holds the message? output() would > mark its element in the array with true, while err() would do so > with false. I would like to *not* deviate further from my original focus. My willingness to address comments that have little to nothing to do with accelerating the interactive rebase has cost me already over a month, with this patch series alone. Do not get me wrong: I think you are right in your assessment that a future caller of the recursive merge might need to be able to tell apart which lines of the output are error messages from the rest, and still retain the order. But then, maybe no future caller will require this. What is certain: right now, no caller requires it, and neither do my queued-up patches to speed up the interactive rebase. And another thing is also certain: *iff* a future caller really will require that fine-grained information of the output, then it will be dramatically easier to implement this on top of the patches we are discussing right now. In short: I hope you agree with me that it is safe to defer the <bool, const char*> tuple implementation to the time when we will *actually* need it. Ciao, Dscho -- 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