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

From

Rubén Justo <rjusto@gmail.com>

Date

Aug 25, 2022, 07:57 UTC

Message-ID

<4d8eb431-dc25-5ea9-14b8-3f84ae0c1561@gmail.com>

In-Reply-To

<55n449n3-71r9-28n9-094q-6r61545r7505@tzk.qr>

Hi,

On 8/19/22 3:05 PM, Johannes Schindelin wrote:

> 
> Rubén, do you want to take this a bit further?
> 

Allow the "-" in setup_revisions:

diff --git a/revision.c b/revision.c
index f4eee11cc8..65e7eb85d8 100644
--- a/revision.c
+++ b/revision.c
@@ -2802,7 +2802,7 @@ int setup_revisions(int argc, const char **argv, 
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])) {
                         int opts;

Then, consider "-" as nth_prior, just like @{-1}:

diff --git a/object-name.c b/object-name.c
index 4d2746574c..87b4c33cce 100644
--- a/object-name.c
+++ b/object-name.c
@@ -934,6 +934,9 @@ static int get_oid_basic(struct repository *r, const 
char *str, int len,
                 }
         }

+        if ((len == 1) && (str[0] == '-'))
+                nth_prior = 1;
+
         /* Accept only unambiguous ref paths. */
         if (len && ambiguous_path(str, len))
                 return -1;

diff --git a/object-name.c b/object-name.c
index 4d2746574c..87b4c33cce 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1420,18 +1423,24 @@ static int interpret_nth_prior_checkout(struct 
repository *r,
         const char *brace;
         char *num_end;

-       if (namelen < 4)
-               return -1;
-       if (name[0] != '@' || name[1] != '{' || name[2] != '-')
-               return -1;
-       brace = memchr(name, '}', namelen);
-       if (!brace)
-               return -1;
-       nth = strtol(name + 3, &num_end, 10);
-       if (num_end != brace)
-               return -1;
-       if (nth <= 0)
-               return -1;
+        if (name[0] == '-' && strchr(".^~:@", name[1])) {
+                nth = 1;
+                brace = name;
+        } else {
+                if (namelen < 4)
+                        return -1;
+                if (name[0] != '@' || name[1] != '{' || name[2] != '-')
+                        return -1;
+                brace = memchr(name, '}', namelen);
+                if (!brace)
+                        return -1;
+                nth = strtol(name + 3, &num_end, 10);
+                if (num_end != brace)
+                        return -1;
+                if (nth <= 0)
+                        return -1;
+        }
+
         cb.remaining = nth;

Two checks needs to be adjusted:

diff --git a/refs.c b/refs.c
index 90bcb27168..0ed9f99ccc 100644
--- a/refs.c
+++ b/refs.c
@@ -198,6 +198,11 @@ static int check_or_sanitize_refname(const char 
*refname, int flags,
                 else
                         return -1;
         }
+
+       if (component_len == 1 && refname[0] == '-') {
+                return -1;
+       }
+

diff --git a/object-name.c b/object-name.c
index 4d2746574c..87b4c33cce 100644
@@ -1684,7 +1693,7 @@ int strbuf_check_branch_ref(struct strbuf *sb, 
const char *name)
          */
         strbuf_splice(sb, 0, 0, "refs/heads/", 11);

-       if (*name == '-' ||
+       if ((*name == '-' && name[1]) ||
             !strcmp(sb->buf, "refs/heads/HEAD"))
                 return -1;


I know this changes open the possibility of having things like "-^2" or 
-@{yesterday} that you said was not desiable. But, why wouldn't we want 
that? Having parse_opt_result to handle that:

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

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


With this changes, all the current uses of "-", with the hacks already 
removed, keep working and fixes the weird cases:

$ git merge branch - other_branch
$ git branch -d branch - other_branch


Also, I've checked that work:
$ git diff -
$ git show -
$ git blame -
and branch -d :-)

I've checked the current tests and added new ones for this, all passes. ie:

diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh
index 4a5758f08a..231457df50 100755
--- a/t/t1505-rev-parse-last.sh
+++ b/t/t1505-rev-parse-last.sh
@@ -40,10 +40,18 @@ test_expect_success '@{-1} works' '
         test_cmp_rev side @{-1}
  '

+test_expect_success '- works' '
+       test_cmp_rev side -
+'
+
  test_expect_success '@{-1}~2 works' '
         test_cmp_rev side~2 @{-1}~2
  '

+test_expect_success '-~2 works' '
+       test_cmp_rev side~2 -~2
+'
+
  test_expect_success '@{-1}^2 works' '
         test_cmp_rev side^2 @{-1}^2
  '

Still needs some work, for example: git log shows "-" in the warning:
$ ../git/git log "-@{10000 minutes ago}"
warning: log for '-' only goes back to Wed, 24 Aug 2022 14:16:17 +0200

What do you think, is it worth the change?

I've created a PR with all the changes and tests.
https://github.com/gitgitgadget/git/pull/1338

Thanks.