git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: mash <mash+git@crossperf.com>
To: Git Mailing List <git@vger.kernel.org>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Vegard Nossum" <vegard.nossum@oracle.com>,
	"Štěpán Němec" <stepnem@gmail.com>,
	"Stefan Beller" <sbeller@google.com>,
	"Vedant Bassi" <sharababy.dev@gmail.com>,
	"Prathamesh Chavan" <pc44800@gmail.com>
Subject: [PATCH v2 GSoC RFC] diff: allow "-" as a short-hand for "last branch"
Date: Thu, 09 Mar 2017 20:26:24 +0000	[thread overview]
Message-ID: <1cm4dm-0007OE-MZ@crossperf.com> (raw)
In-Reply-To: <1clZj4-0006vN-9q@crossperf.com>

Just like "git merge -" is a short-hand for "git merge @{-1}" to
conveniently merge the previous branch, "git diff -" is a short-hand for
"git diff @{-1}" to conveniently diff against the previous branch.

Allow the usage of "-" in the dot dot notation to allow the use of
"git diff -..HEAD^" as a short-hand for "git diff @{-1}..HEAD^".

Signed-off-by: mash <mash+git@crossperf.com>
---
Add tests to confirm that passing in this short-hand from stdin works.

Handling the dash in sha1_name:get_sha1_basic is not an issue but git was
designed with the dash in mind for options not for this weird short-hand so as
long as there's no decision made that git should actually have this short-hand
everywhere it does not seem like a good idea to change anything in there
because it would probably have unwanted side-effects.

For example for now just handle_revision_arg was modified which is mainly used
by git diff but also used in builtin/pack-objects.c:get_object_list#2785 which
is terrible since this may have already introduced an unwanted the side-effect.

Bypassing the whatever starts with a dash is always an option filters is not a
nice thing to do either.

Overall I see the benefit of the dash short-hand when doing checkouts since
it's very similar to "cd -" and switching between two branches is something one
would commonly do but for every git command where executing it twice results
into the same action achieving the same result twice it seems like a not that
useful short-hand.

Example:
master# g co maint
maint# g co -  # Switch to master
master# g co -  # Switch to maint (different result)
maint# g d -  # diff master..HEAD
maint# g d -  # diff master..HEAD (same result - less useful)

This is obviously only my own option.
Because of what was stated above I've now marked this as open for discussion
since I'm currently not convinced that applying the patch is a good idea.

 revision.c           | 22 +++++++++++++++--
 t/t4063-diff-last.sh | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 2 deletions(-)
 create mode 100755 t/t4063-diff-last.sh

diff --git a/revision.c b/revision.c
index b37dbec..c331bd5 100644
--- a/revision.c
+++ b/revision.c
@@ -1439,6 +1439,7 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
 	const char *arg = arg_;
 	int cant_be_filename = revarg_opt & REVARG_CANNOT_BE_FILENAME;
 	unsigned get_sha1_flags = 0;
+	static const char previous_branch[] = "@{-1}";
 
 	flags = flags & UNINTERESTING ? flags | BOTTOM : flags & ~BOTTOM;
 
@@ -1457,6 +1458,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
 
 		if (!*next)
 			next = head_by_default;
+		else if (!strcmp(next, "-"))
+			next = previous_branch;
 		if (dotdot == arg)
 			this = head_by_default;
 		if (this == head_by_default && next == head_by_default &&
@@ -1469,6 +1472,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
 				*dotdot = '.';
 				return -1;
 			}
+		} else if (!strcmp(this, "-")) {
+			this = previous_branch;
 		}
 		if (!get_sha1_committish(this, from_sha1) &&
 		    !get_sha1_committish(next, sha1)) {
@@ -1568,6 +1573,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
 	if (revarg_opt & REVARG_COMMITTISH)
 		get_sha1_flags = GET_SHA1_COMMITTISH;
 
+	if (!strcmp(arg, "-"))
+		arg = previous_branch;
 	if (get_sha1_with_context(arg, get_sha1_flags, sha1, &oc))
 		return revs->ignore_missing ? 0 : -1;
 	if (!cant_be_filename)
@@ -1578,6 +1585,15 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
 	return 0;
 }
 
+/*
+ * Check if the argument is supposed to be a revision argument instead of an
+ * option even though it starts with a dash.
+ */
+static int is_revision_arg(const char *arg)
+{
+	return *arg == '\0' || starts_with(arg, "..");
+}
+
 struct cmdline_pathspec {
 	int alloc;
 	int nr;
@@ -1621,7 +1637,9 @@ static void read_revisions_from_stdin(struct rev_info *revs,
 				seen_dashdash = 1;
 				break;
 			}
-			die("options not supported in --stdin mode");
+			if (!is_revision_arg(sb.buf + 1)) {
+				die("options not supported in --stdin mode");
+			}
 		}
 		if (handle_revision_arg(sb.buf, revs, 0,
 					REVARG_CANNOT_BE_FILENAME))
@@ -2205,7 +2223,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	read_from_stdin = 0;
 	for (left = i = 1; i < argc; i++) {
 		const char *arg = argv[i];
-		if (*arg == '-') {
+		if (*arg == '-' && !is_revision_arg(arg + 1)) {
 			int opts;
 
 			opts = handle_revision_pseudo_opt(submodule,
diff --git a/t/t4063-diff-last.sh b/t/t4063-diff-last.sh
new file mode 100755
index 0000000..dc28f9d
--- /dev/null
+++ b/t/t4063-diff-last.sh
@@ -0,0 +1,68 @@
+#!/bin/sh
+
+test_description='diff against last branch'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo hello >world &&
+	git add world &&
+	git commit -m initial &&
+	git branch other &&
+	echo "hello again" >>world &&
+	git add world &&
+	git commit -m second
+'
+
+test_expect_success '"diff -" does not work initially' '
+	test_must_fail git diff -
+'
+
+test_expect_success '"diff -" diffs against previous branch' '
+	git checkout other &&
+
+	cat <<-\EOF >expect &&
+	diff --git a/world b/world
+	index c66f159..ce01362 100644
+	--- a/world
+	+++ b/world
+	@@ -1,2 +1 @@
+	 hello
+	-hello again
+	EOF
+
+	git diff - >out &&
+	test_cmp expect out
+'
+
+test_expect_success '"diff -" arguments from stdin' '
+	echo "-" | git diff --stdin >out &&
+	test_cmp expect out
+'
+
+test_expect_success '"diff -.." diffs against previous branch' '
+	git diff -.. >out &&
+	test_cmp expect out
+'
+
+test_expect_success '"diff -.." arguments from stdin' '
+	echo "-.." | git diff --stdin >out &&
+	test_cmp expect out
+'
+
+test_expect_success '"diff ..-" diffs inverted' '
+	cat <<-\EOF >expect &&
+	diff --git a/world b/world
+	index ce01362..c66f159 100644
+	--- a/world
+	+++ b/world
+	@@ -1 +1,2 @@
+	 hello
+	+hello again
+	EOF
+
+	git diff ..- >out &&
+	test_cmp expect out
+'
+
+test_done
-- 
2.9.3

  reply	other threads:[~2017-03-09 20:26 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-08  9:50 [PATCH] diff: allow "-" as a short-hand for "last branch" mash
2017-03-09 20:26 ` mash [this message]
     [not found]   ` <20170310034106.GB1984@instance-1.c.mfqp-source.internal>
2017-03-10  4:52     ` [PATCH v2 GSoC RFC] " mash
2017-03-10  5:00       ` Siddharth Kannan
  -- strict thread matches above, loose matches on Subject: below --
2017-03-10  4:59 Siddharth Kannan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1cm4dm-0007OE-MZ@crossperf.com \
    --to=mash+git@crossperf.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pc44800@gmail.com \
    --cc=sbeller@google.com \
    --cc=sharababy.dev@gmail.com \
    --cc=stepnem@gmail.com \
    --cc=vegard.nossum@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).