On 04/09, Eric Sunshine wrote: > On Mon, Apr 9, 2018 at 3:30 PM, Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote: > > On 04/08, Eric Sunshine wrote: > >> As with Junio, I'm fine with this hidden option (for now), however, I > >> think you can take this a step further. Rather than having a (hidden) > >> git-reset option which suppresses "HEAD is now at...", instead have a > >> (hidden) option which augments the message. For example, > >> --new-head-desc="New worktree" would make it output "New worktree HEAD > >> is now at...". Changes to builtin/reset.c to support this would hardly > >> be larger than the changes you already made. > > > > Something else I just noticed that may make this a worse solution is > > that this breaks the sentence in two pieces for translators. I guess > > we could somehow get the "New worktree" part of the option translated, > > but that still means that if some language would require to move parts > > of the sentence around that would be less than ideal for translation. > > Good point. > > One solution would be to have the new hidden option replace the string > entirely: --new-head-msg="New worktree HEAD is now at %s", which would > allow translators to deal with the entire sentence. Even clearer would > be to drop "now", as in "New worktree HEAD is at %s". (Default in > reset.c would still be "HEAD is now at %s", of course.) > > Another solution would be not to augment the "HEAD is now at..." > message at all. I realize that that augmentation was one of the > original motivations for this patch series, but with the upcoming > restoration of the "Preparing worktree" message: My original motivation of the series was to just make the new dwim work :) Because that's adding some magic, the secondary motivation became improving the UI, to help users see which dwim was used. I felt like this was going to be one of those improvements, especially after we get rid of the "Preparing ..." line. I do however like your suggestion of the "Preparing worktree (_branch disposition_)", as that doesn't add more lines to the output, while still giving a good indication of what exactly is happening. At that point just showing "HEAD is now at ..." is fine by me, and doesn't require adding the hidden flag to 'git reset'. So I'm happy just dropping the change in the message here, which will simplify things. Thanks for the suggestion! > Preparing worktree (_branch disposition_) > HEAD is now at ... > > it seems clear enough (at least to me) from the context introduced by > the "Preparing..." message that "HEAD is now at..." refers to HEAD in > the worktree. (But that's just my opinion.) > > > Would factoring out what we have in 'print_new_head_line()' into some > > common code, maybe in 'pretty.c', and still doing the printing from > > here be a reasonable tradeoff? > > Isn't that getting uglier again? Not only would you have to publish > that function, but you'd still need the hidden git-reset > --show-new-head-line option. > > Also, you'd end up calling that function from within low-level worker > worktree.c:add_worktree(), thus making it take on UI duties, which > would be nice to avoid if possible. (Unfortunately, the alternate idea > of having worktree.c:add() handle this UI task doesn't quite work > since add_worktree() doesn't return sufficient information for add() > to know whether or not it should print "HEAD is now at..."; however, > add_worktree() could be augmented to return more information.) > > So, I don't presently have a good answer. I'm partial to the idea of > not augmenting "HEAD is now at...", partly because context makes it > relatively clear that it applies to the new worktree and partly > because it's simpler (less implementation work for you) to leave it as > is. If that choice isn't desirable, then next best would be for > --new-head-msg= to replace the entire "HEAD is now at..." string > rather than augmenting it.
