Subject: Re: [PATCH 3/4] read-tree.c: rework UI when merging no trees

From: Jean-Noël AVILA <[email-masked]>
Date: Wed, 03 May 2017 20:39:50 +0200

Le mercredi 3 mai 2017, 10:04:01 CEST Jonathan Nieder a écrit :
> Hi,
> 
> Jean-Noel Avila wrote:
> > Subject: read-tree.c: rework UI when merging no trees
> 
> nit: this is about user-facing behavior, not an implementation detail,
> so the part before the colon can be the command that changed
> (read-tree:).
> 
> nit: the word "rework" is dangerous in a commit message in the same
> way as the word "fix" --- it stands for "make better", in a vague way
> that leaves the reader guessing about how.  Usually a more specific
> description can work better.
> 

In fact, this patch is two fold:

 * reword the question in the die() call. I realize now that when passed to 
die(), the string is prepended with "fatal:". That's an hint that the question 
does not require a reply, but  ruling out any doubt would be better.
 * rework the local logic which was inherited from history. This is 
functionally equivalent to the previous version, just cleaner.

> > The initial test was inherited from a previous commit, but it is no
> > longer needed, given the following switch case. Moreover, the question
> > sentence ending the program has been replace by an assertative one.
> > 
> > Signed-off-by: Jean-Noel Avila <[email-masked]>
> 
> This can have a simpler, short-and-sweet motivation:
> 
> 	read-tree -m: make error message for merging 0 trees less smart-alecky
> 
> 	"git read-tree -m" requires a tree argument to name the tree to be
> 	merged in.  Git uses a cutesy error message to say so and why:
> 
> 		$ git read-tree -m
> 		warning: read-tree: emptying the index with no arguments is deprecated;
> use --empty fatal: just how do you expect me to merge 0 trees?
> 		$ git read-tree -m --empty
> 		fatal: just how do you expect me to merge 0 trees?
> 
> 	When lucky, that could produce an ah-hah moment for the user, but it's
> 	more likely to irritate and distract them.
> 
> 	Instead, tell the user plainly that the tree argument is required. Also
> 	document this requirement in the git-read-tree(1) manpage where there is
> 	room to explain it in a more straightforward way.
> 

Thank you very much for this message! May I s-o-b you?

As hinted, I'll add the documentation part. ;-)

> Unfortunately both 'git read-tree -h' and 'git read-tree --help' say nothing
> about this.  Ideas for wording there?

Next pach series will propose this.

> 
> Thanks and hope that helps,
> Jonathan