Re: [PATCH v3] Advertise the ability to abort a commit

[Jeff King <peff@peff.net>]

On Thu, Jul 31, 2008 at 12:55:39PM +0200, Petr Baudis wrote:

> >  		if (cleanup_mode == CLEANUP_ALL)
> > -			fprintf(fp, "not be included)\n");
> > +			fprintf(fp,
> > +				" Lines starting\n"
> > +				"# with '#' will be ignored, and an empty"
> > +				" message aborts the commit.\n");
> >  		else /* CLEANUP_SPACE, that is. */
> > -			fprintf(fp, "be kept.\n"
> > -				"# You can remove them yourself if you want to)\n");
> > +			fprintf(fp,
> > +				" Lines starting\n"
> > +				"# with '#' will be kept; you may remove them"
> > +				" yourself if you want to.\n"
> > +				"# An empty message aborts the commit.\n");
> >  		if (only_include_assumed)
> >  			fprintf(fp, "# %s\n", only_include_assumed);
> >  
> 
> This is rather funny-looking; you print _one_ fragment of the common
> string by a common fprintf, but then repeat _second_ fragment of the
> still-common string in a per-case fprintf. Can't we at least split this
> on the line boundary, if not do something loosely like this?

I just broke it by sentence, thinking that followed the semantics more
clearly (i.e., the first fprintf says one thing, then the second says
another; however, we must say the second one differently depending on
the case). I almost just split the whole paragraph by cleanup case,
allowing each to be worded and wrapped as most appropriate.

> 		fprintf(fp,
> 			"\n"
> 			"# Please enter the commit message for your "
> 			"changes. Lines starting\n"
> 			"# with a '#' will be %s "
> 			"and an empty message aborts the commit\n",
> 			cleanup_mode == CLEANUP_ALL ? "ignored,"
> 			/* CLEANUP_SPACE */ : "kept (you may remove them "
> 				"yourself if you want to)\n#");

I did something like that before submitting, but decided against it
because:

  - I found mine more readable, since it is hard to see in yours exactly
    where there will be a linebreak.

  - I actually changed the phrasing for the second one. Since we
    introduce another clause into the sentence in the CLEANUP_SPACE
    case, it makes sense to start another sentence for the final point.

-Peff