[RFC] revision: Don't let ^<rev> cancel out the default <rev>

[RFC] revision: Don't let ^<rev> cancel out the default <rev>

[Andreas Gruenbacher]

Some commands like 'log' default to HEAD if no other revisions are
specified on the command line or otherwise.  Unfortunately, excludes
(^<rev>) cancel out that default, so when a command only excludes
revisions (e.g., 'git log ^origin/master'), the command will not produce
any result.

If all the specified revisions are excludes, it seems more useful to
stick with the default revision instead.

This makes writing wrappers that exclude certain revisions much easier:
for example, a simple alias l='git log ^origin/master' will show the
revisions between origin/master and HEAD by default, and 'l foo' will
show the revisions between origin/master and foo, as you would usually
expect.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 revision.c     | 18 ++++++++++++++----
 t/t4202-log.sh |  6 ++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/revision.c b/revision.c
index de4dce600..c2c51bd5d 100644
--- a/revision.c
+++ b/revision.c
@@ -1158,6 +1158,18 @@ static void add_rev_cmdline(struct rev_info *revs,
 	info->nr++;
 }
 
+static int has_interesting_revisions(struct rev_info *revs)
+{
+	struct rev_cmdline_info *info = &revs->cmdline;
+	unsigned int n;
+
+	for (n = 0; n < info->nr; n++) {
+		if (!(info->rev[n].flags & UNINTERESTING))
+			return 1;
+	}
+	return 0;
+}
+
 static void add_rev_cmdline_list(struct rev_info *revs,
 				 struct commit_list *commit_list,
 				 int whence,
@@ -2318,7 +2330,7 @@ static void NORETURN diagnose_missing_default(const char *def)
  */
 int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct setup_revision_opt *opt)
 {
-	int i, flags, left, seen_dashdash, read_from_stdin, got_rev_arg = 0, revarg_opt;
+	int i, flags, left, seen_dashdash, read_from_stdin, revarg_opt;
 	struct argv_array prune_data = ARGV_ARRAY_INIT;
 	const char *submodule = NULL;
 
@@ -2401,8 +2413,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 			argv_array_pushv(&prune_data, argv + i);
 			break;
 		}
-		else
-			got_rev_arg = 1;
 	}
 
 	if (prune_data.argc) {
@@ -2431,7 +2441,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 		opt->tweak(revs, opt);
 	if (revs->show_merge)
 		prepare_show_merge(revs);
-	if (revs->def && !revs->pending.nr && !revs->rev_input_given && !got_rev_arg) {
+	if (revs->def && !revs->rev_input_given && !has_interesting_revisions(revs)) {
 		struct object_id oid;
 		struct object *object;
 		struct object_context oc;
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 153a50615..e6670859c 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -213,6 +213,12 @@ test_expect_success 'git show <commits> leaves list of commits as given' '
 	test_cmp expect actual
 '
 
+printf "sixth\nfifth\n" > expect
+test_expect_success '^<rev>' '
+	git log --pretty="tformat:%s" ^HEAD~2 > actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'setup case sensitivity tests' '
 	echo case >one &&
 	test_tick &&
-- 
2.17.1

Re: [RFC] revision: Don't let ^<rev> cancel out the default <rev>

[Junio C Hamano]

Andreas Gruenbacher <agruenba@redhat.com> writes:

> Some commands like 'log' default to HEAD if no other revisions are
> specified on the command line or otherwise.  Unfortunately, excludes
> (^<rev>) cancel out that default, so when a command only excludes
> revisions (e.g., 'git log ^origin/master'), the command will not produce
> any result.
>
> If all the specified revisions are excludes, it seems more useful to
> stick with the default revision instead.
>
> This makes writing wrappers that exclude certain revisions much easier:
> for example, a simple alias l='git log ^origin/master' will show the
> revisions between origin/master and HEAD by default, and 'l foo' will
> show the revisions between origin/master and foo, as you would usually
> expect.

That is a _huge_ departure from the behaviour established for the
past 10 years, but it would certainly be more useful.

As long as we can prove that that a command line with only negative
revs is absolutely useless, the backward incompatibility may be OK
to swallow, especially for commands like "git log" that implicitly
use "--default HEAD", as they are meant for human consumption, and
not for scripts.

I am not offhand 100% sure that a rev list with only negative revs
is totally useless, though.

Re: [RFC] revision: Don't let ^<rev> cancel out the default <rev>

[Jeff King]

On Wed, Aug 29, 2018 at 02:03:34PM -0700, Junio C Hamano wrote:

> Andreas Gruenbacher <agruenba@redhat.com> writes:
> 
> > Some commands like 'log' default to HEAD if no other revisions are
> > specified on the command line or otherwise.  Unfortunately, excludes
> > (^<rev>) cancel out that default, so when a command only excludes
> > revisions (e.g., 'git log ^origin/master'), the command will not produce
> > any result.
> >
> > If all the specified revisions are excludes, it seems more useful to
> > stick with the default revision instead.
> >
> > This makes writing wrappers that exclude certain revisions much easier:
> > for example, a simple alias l='git log ^origin/master' will show the
> > revisions between origin/master and HEAD by default, and 'l foo' will
> > show the revisions between origin/master and foo, as you would usually
> > expect.
> 
> That is a _huge_ departure from the behaviour established for the
> past 10 years, but it would certainly be more useful.
> 
> As long as we can prove that that a command line with only negative
> revs is absolutely useless, the backward incompatibility may be OK
> to swallow, especially for commands like "git log" that implicitly
> use "--default HEAD", as they are meant for human consumption, and
> not for scripts.
> 
> I am not offhand 100% sure that a rev list with only negative revs
> is totally useless, though.

Yeah, I'm uneasy that somebody is relying on the current behavior,
especially for scripting where we often do something like:

  git rev-list $new --not --all

and an empty $new really should return an empty result (that's our usual
connectivity check, but I'd imagine some pre-receive hooks may end up
doing similar things). For rev-list I think you'd have to specify
--default to trigger this behavior, so that helps. But it still makes me
nervous.

I'm _less_ uneasy with it for git-log, though I think people do have a
habit of scripting around it (because it knows some tricks that rev-list
doesn't).

Given that the mentioned use case was writing wrappers, could we hide
this behind:

  git config alias.l 'git log --default-on-negative ^origin/master'

That's obviously less convenient to type, but I think it would usually
be part of scripted calls (otherwise you'd know already that you have no
positive refs and would just add ".." or HEAD or whatever).

-Peff