From: Rubén Justo Date: Sat, 13 Aug 2022 09:19:40 GMT Subject: Re: [PATCH] branch: allow "-" as a short-hand for "previous branch" Message-ID: In-Reply-To: On Mon, Aug 8, 2022 at 6:06 PM Junio C Hamano > wrote: > > Johannes Schindelin > writes: > > > @@ -1420,6 +1420,12 @@ static int interpret_nth_prior_checkout(struct repository *r, > >       const char *brace; > >       char *num_end; > > > > +     if (namelen == 1 && *name == '-') { > > +             brace = name; > > +             nth = 1; > > +             goto find_nth_checkout; > > +     } > > + > >       if (namelen < 4) > >               return -1; > >       if (name[0] != '@' || name[1] != '{' || name[2] != '-') > > If a solution along this line works, it would be far cleaner design > than the various hacks we have done in the past, noticing "-" and > replacing with "@{-1}".  For one thing, we wouldn't be receiving a > "-" from the end user on the command line and in response say @{-1} > does not make sense in the context in an error message. That alone > makes the above approach to deal with it at the lowest level quite > attractive. > > In the list archive, however, you may be able to find a few past > discussions on why this is not a good idea (some of which I may no > longer agree with).  One thing that still worries me a bit is that > we often disambiguate the command line arguments by seeing "is this > (still) a rev, or is this a file, or can it be interpreted as both?" > and "-" is not judged to be a "rev", IIRC. > > Luckily, not many commands we have take "-" as if it were a file and > make it read from the standard input stream, but if there were (or > if we were to add a command to behave like so), treating "-" to mean > the same thing as "@{-1}" everywhere may require the "does this look > like a rev?"  heuristics (which is used by the "earlier ones must be > rev and not file, later ones must be file and cannot be interpreted > as rev, for you to omit '--' from the command line" logic) to be > taught that a lone "-" can be a rev. > > So it is quite a lot of thing that the new code needs to get right > before getting there. Agree. To make a substitution in the command line and to consider "-" in interpret_nth_prior_checkout, I see them as two very different games. Previous to this, I thought about making also a "git diff -", https://github.com/gitgitgadget/git/pull/1314 . Suddenly there was 5 commands with this substitution (checkout, merge, rebase, branch, diff) so I follow a little the path now Johannes suggests, making the substitution "- ~ @{-1}" deep in the system. For me, the implications, error cases, test cases... to consider was not worth the change to get what I was looking for: align the workflow "checkout/merge/branch -d". Also discarded the "git diff -" change, because of so many flags and conditions "diff" has. So I only sent the "branch -" patch.