Re: [PATCH] branch: allow "-" as a short-hand for "previous branch"

["Rubén Justo" <rjusto@gmail.com>]

On 8/25/22 6:23 PM, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
> 
>> struct rev_info *revs, struct s
>>                  revarg_opt |= REVARG_CANNOT_BE_FILENAME;
>>          for (left = i = 1; i < argc; i++) {
>>                  const char *arg = argv[i];
>> -               if (!seen_end_of_options && *arg == '-') {
>> +               if (!seen_end_of_options && *arg == '-' &&
>> !strchr(".^~:@", arg[1])) {
> 
> Yuck.  I really didn't want to see that strchr() or any other logic
> that _knows_ what letter is allowed after a "rev".  It will be
> impossible to keep it in sync with the real code paths that needs to
> know and parses these syntactical constructs, and folks new to the
> codebase will never be able to tell at a first glance if the above
> is sufficient (or what the strchr() is doing).
> 

Some logic is needed to disambiguate from options. It is difficult than 
that set of chars changes, they are all around the code. And if any new 
is added should be reviewed and hopefully some test will be broken.

Maybe a more centralized approach?

diff --git a/parse-options.c b/parse-options.c
index 2757bd94c1..303854e8a4 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -740,7 +741,7 @@ enum parse_opt_result parse_options_step(struct 
parse_opt_ctx_t *ctx,
                     ctx->argc != ctx->total)
                         break;

-               if (*arg != '-' || strchr(".^~:@", arg[1])) {
+               if (*arg != '-' || 
!check_refchar_component_special(arg[1])) {
                         if (parse_nodash_opt(ctx, arg, options) == 0)
                                 continue;
                         if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)


diff --git a/refs.c b/refs.c
index 0ed9f99ccc..5327a8ec1f 100644
--- a/refs.c
+++ b/refs.c
@@ -159,6 +159,32 @@ static int check_refname_component(const char 
*refname, int *flags,
         return cp - refname;
  }

+int check_refchar_component_special(char refchar)
+{
+        int ch = refchar & 255;
+        unsigned char disp = refname_disposition[ch];
+
+        switch (disp) {
+        case 1:
+                /* end of component */
+                return 0;
+        case 2:
+                /* ".." components */
+                return 0;
+        case 3:
+                /* "@{" components */
+                return 0;
+        case 4:
+                /* forbidden char */
+                return 0;
+        case 5:
+                /* pattern */
+                return 0;
+        }
+
+        return -1;
+}
+
  static int check_or_sanitize_refname(const char *refname, int flags,
                                      struct strbuf *sanitized)
  {