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

From

Rubén Justo <rjusto@gmail.com>

Date

Aug 25, 2022, 19:50 UTC

Message-ID

<b5076054-1be6-1dc5-868a-05005a594b0d@gmail.com>

In-Reply-To

<xmqqo7w82sx1.fsf@gitster.g>

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).
> 

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)
  {