All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH/RFC v2] WIP: log: allow "-" as a short-hand for "previous branch"
@ 2017-02-07 20:51 Siddharth Kannan
  0 siblings, 0 replies; only message in thread
From: Siddharth Kannan @ 2017-02-07 20:51 UTC (permalink / raw)
  To: git
  Cc: gitster, pranit.bauva, Matthieu.Moy, peff, pclouds, sandals,
	Siddharth Kannan

Teach revision.c:setup_revisions that an argument starting with "-" can be an
argument also. `left` variable needs to be incremented only when the supplied
arg is neither an argument, nor an option.

Teach sha1_name.c:get_sha1_1 that "-" is equivalent to "@{-1}"

Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
---
I have run the test suite locally and on Travis CI. [1]

Whenever the argument begins with a "-" then the function "handle_revision_arg" 
is called twice. I can fix this using a variable that would store whether the
function has been called earlier or not. For doing that I have to investigate
some more on what the valid return values for "handle_revision_arg" are. (Or
a simpler approach would be to use an integer flag, this would also not be
affected if in case "handle_revision_arg" is changed in the future)

I have also written a very basic test for git-log only. I have based this on the
tests that were added in 696acf4 (checkout: implement "-" abbreviation, add docs
and tests, 2009-01-17). It tests revisions, revision ranges, and open-ended
revision ranges (where the start or the finish argument is inferred) If the
code in this patch is okay, or close to okay, then please tell me if further
tests need to be added for git-log and/or other commands.

This change touches a few commands, other than log. notably, git-format-patch,
git-whatchanged and git-show, all of which are defined inside builtin/log.c. In
general, it affects commands that call setup_revisions at some point in their
codepath. (eg: git-diff-index)

Thanks a lot, Junio, for your comments on the previous version of this patch and
further discussion on that thread!

[1]: https://travis-ci.org/icyflame/git/builds/199350136

 revision.c               |  9 +++++--
 sha1_name.c              |  5 ++++
 t/t4214-log-shorthand.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100755 t/t4214-log-shorthand.sh

diff --git a/revision.c b/revision.c
index b37dbec..e14f62c 100644
--- a/revision.c
+++ b/revision.c
@@ -2206,7 +2206,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	for (left = i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 		if (*arg == '-') {
-			int opts;
+			int opts, args;
 
 			opts = handle_revision_pseudo_opt(submodule,
 						revs, argc - i, argv + i,
@@ -2234,7 +2234,12 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 			}
 			if (opts < 0)
 				exit(128);
-			continue;
+
+			args = handle_revision_arg(arg, revs, flags, revarg_opt);
+			if (args)
+				continue;
+			else
+				--left;
 		}
 
 
diff --git a/sha1_name.c b/sha1_name.c
index 73a915f..d774e46 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -947,6 +947,11 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned l
 	if (!ret)
 		return 0;
 
+	if (!strcmp(name, "-")) {
+		name = "@{-1}";
+		len = 5;
+	}
+
 	ret = get_sha1_basic(name, len, sha1, lookup_flags);
 	if (!ret)
 		return 0;
diff --git a/t/t4214-log-shorthand.sh b/t/t4214-log-shorthand.sh
new file mode 100755
index 0000000..95cf2d4
--- /dev/null
+++ b/t/t4214-log-shorthand.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+test_description='log can show previous branch using shorthand - for @{-1}'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo hello >world &&
+	git add world &&
+	git commit -m initial &&
+	echo "hello second time" >>world &&
+	git add world &&
+	git commit -m second &&
+	echo "hello other file" >>planet &&
+	git add planet &&
+	git commit -m third &&
+	echo "hello yet another file" >>city &&
+	git add city &&
+	git commit -m fourth
+'
+
+test_expect_success '"log -" should not work initially' '
+	test_must_fail git log -
+'
+
+test_expect_success '"log -" should work' '
+	git checkout -b testing-1 master^ &&
+	git checkout -b testing-2 master~2 &&
+	git checkout master &&
+
+	git log testing-2 >expect &&
+	git log - >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'revision range should work when one end is left empty' '
+	git checkout testing-2 &&
+	git checkout master &&
+	git log ...@{-1} > expect.first_empty &&
+	git log @{-1}... > expect.last_empty &&
+	git log ...- > actual.first_empty &&
+	git log -... > actual.last_empty &&
+	test_cmp expect.first_empty actual.first_empty &&
+	test_cmp expect.last_empty actual.last_empty
+'
+
+test_expect_success 'symmetric revision range should work when both ends are given' '
+	git checkout testing-2 &&
+	git checkout master &&
+	git log -...testing-1 >expect &&
+	git log testing-2...testing-1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'asymmetric revision range should work when both ends are given' '
+	git checkout testing-2 &&
+	git checkout master &&
+	git log -..testing-1 >expect &&
+	git log testing-2..testing-1 >actual &&
+	test_cmp expect actual
+'
+test_done
-- 
2.1.4


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2017-02-07 20:51 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-07 20:51 [PATCH/RFC v2] WIP: log: allow "-" as a short-hand for "previous branch" Siddharth Kannan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.