Re: [PATCH 2/2] push: add an advice on unqualified <dst> push

[Jeff King <peff@peff.net>]

On Wed, Oct 10, 2018 at 11:23:25PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > I wonder if we could reword the first paragraph to be a little less
> > confusing, and spell out what we tried already. E.g., something like:
> >
> >   The destination you provided is not a full refname (i.e., starting
> >   with "ref"). Git tried to guess what you meant by:
> >
> >     - looking for a matching branch or tag on the remote side
> >
> >     - looking at the refname of the local source
> >
> >   but neither worked.
> >
> >   The <src> part of the refspec is a commit object.
> >   Did you mean...
> 
> Yeah that makes sense. I was trying to avoid touching the existing
> wording to make this more surgical, but you came up with it, and since
> you don't like it I'll just change that too.

I certainly know the feeling of trying to avoid wording bikeshed
discussions. But in this instance, please feel free to aggressively
rewrite that old message. ;)

What I wrote above was off-the-cuff, and I also do not mind if you use
it as a starting point to make improvements (or take it wholesale if you
really like it).

> > I think it would probably be OK to put the first paragraph in its own
> > variable. I know we try to avoid translation lego, but I'd think
> > paragraphs are separate units. Or are you worried about how to get them
> > into the same advise() call? I don't know that we need to, but we could
> > also plug one into the other with a "%s" (and leave a translator note).
> 
> To be honest from being on the code side of a much bigger i18n effort
> (the MediaWiki/WikiMedia software) back in the early days of my career I
> just do this sort of thing reflexively, because from experience when I
> started trying to simplify stuff by making assumptions I was wrong every
> time.
> [...]

OK, I'm happy to defer to your judgement here. I have very little
translation experience myself.

> > Can we just do it as:
> >
> >   if (advice_push_ambiguous_ref_name) {
> > 	struct object_id oid;
> > 	enum object_type type;
> >
> > 	if (get_oid(...))
> > 	   etc...
> >   }
> >
> > instead? That pushes your indentation one level in, but I think the
> > whole conditional body might be cleaner in a helper function anyway.
> 
> I started out with that and found myself really constrained by the 72
> char ceiling which I'm already smashing through with these ~95 character
> lines (but at least it's under 100!). But sure, we can do with 8 more.

That's why I suggested the helper function. :) I'm also not opposed to
pulling messages out to static file-level variables, even if they're
only used once. Sometimes it's nice to have them left-aligned (or close
to it) to see how they'll actually look in a terminal.

-Peff