On Fri, May 22, 2015 at 7:14 AM, Johannes Schindelin <[email protected]> wrote: > Hi Paul, > > On 2015-05-22 15:48, Paul Tan wrote: >> On Wed, May 20, 2015 at 12:27 AM, Junio C Hamano <[email protected]> wrote: >>> Johannes Schindelin <[email protected]> writes: >>> >>>>>> - fprintf(stderr, >>>>>> - _("There are no candidates for merging among the >>>>>> refs that you just fetched.\n" >>>>>> - "Generally this means that you provided a >>>>>> wildcard refspec which had no\n" >>>>>> - "matches on the remote end.\n")); >>>>>> + if (opt_rebase) >>>>>> + fputs(_("There is no candidate for rebasing >>>>>> against among the refs that you just fetched."), stderr); >>>>> >>>> The puts() function appends a <newline> while fputs() does not. >>> >>> Yup, so this update makes the command spew unterminated lines, which >>> not something intended... >> >> Ugh >< Will put the "\n" back. >> >> And yes, I used fputs() because it seems wasteful to use fprintf() >> which will scan the string looking for any '%' formatting units, when >> we know there aren't. >> >> I will also update 05/14 to use fputs() as well where appropriate. > > I believe the common thinking is that consistency beats speed in error > messages, so it would be easier to read and maintain the code if all of those > error messages were just using `fprintf(stderr, ...);`. It's not as if we > spit out hundreds of thousands of error messages per second where that `%s` > parsing would hurt ;-) > > Ciao, > Dscho
As soon as we spit out one error message, the speed game is over anyway. (IO is slow, and I believe in the error case correctness is the most critical thing to get right, so no need to pay attention to performance). Though I don't mind having a fputs/fprintf mixture. I just happened to work on parts of code without any fputs before, that's why I brought up this discussion. Actually I think I'd prefer fputs when there is no %. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
