[RFC] revision: Add --sticky-default option

[RFC] revision: Add --sticky-default option

[Andreas Gruenbacher]

Hi,

here's a long-overdue update of my proposal from August 29:

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

Does this look more acceptable that my first shot?

Thanks,
Andreas

--

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

With the --sticky-default option, the default becomes more "sticky" and
is no longer canceled out by excludes.

This is useful in wrappers that exclude certain revisions: for example,
a simple alias l='git log --sticky-default ^origin/master' will show the
revisions between origin/master and HEAD when invoked without arguments,
and 'l foo' will show the revisions between origin/master and foo.

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

diff --git a/revision.c b/revision.c
index e18bd530e..6c93ec74b 100644
--- a/revision.c
+++ b/revision.c
@@ -1159,6 +1159,25 @@ static void add_rev_cmdline(struct rev_info *revs,
 	info->nr++;
 }
 
+static int has_revisions(struct rev_info *revs)
+{
+	struct rev_cmdline_info *info = &revs->cmdline;
+
+	return info->nr != 0;
+}
+
+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,
@@ -2132,6 +2151,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--children")) {
 		revs->children.name = "children";
 		revs->limited = 1;
+	} else if (!strcmp(arg, "--sticky-default")) {
+		revs->sticky_default = 1;
 	} else if (!strcmp(arg, "--ignore-missing")) {
 		revs->ignore_missing = 1;
 	} else if (!strcmp(arg, "--exclude-promisor-objects")) {
@@ -2319,7 +2340,8 @@ 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, got_rev_arg = 0, revarg_opt;
+	int i, flags, left, seen_dashdash, revarg_opt;
+	int cancel_default;
 	struct argv_array prune_data = ARGV_ARRAY_INIT;
 	const char *submodule = NULL;
 
@@ -2401,8 +2423,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 +2451,10 @@ 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) {
+	cancel_default = revs->sticky_default ?
+			 has_interesting_revisions(revs) :
+			 has_revisions(revs);
+	if (revs->def && !revs->rev_input_given && !cancel_default) {
 		struct object_id oid;
 		struct object *object;
 		struct object_context oc;
diff --git a/revision.h b/revision.h
index 2b30ac270..570fa1a6d 100644
--- a/revision.h
+++ b/revision.h
@@ -92,6 +92,7 @@ struct rev_info {
 
 	unsigned int early_output;
 
+	unsigned int	sticky_default:1;
 	unsigned int	ignore_missing:1,
 			ignore_missing_links:1;
 
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 153a50615..9517a65da 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 '--sticky-default ^<rev>' '
+	git log --pretty="tformat:%s" --sticky-default ^HEAD~2 > actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'setup case sensitivity tests' '
 	echo case >one &&
 	test_tick &&
-- 
2.17.2

Re: [RFC] revision: Add --sticky-default option

[Jeff King]

On Tue, Oct 16, 2018 at 11:24:38PM +0200, Andreas Gruenbacher wrote:

> here's a long-overdue update of my proposal from August 29:
> 
>   [RFC] revision: Don't let ^<rev> cancel out the default <rev>
> 
> Does this look more acceptable that my first shot?

I think it's going in the right direction.

The name "--sticky-default" did not immediately make clear to me what it
does. Is there some name that would be more obvious?

> Some commands like 'log' default to HEAD if no other revisions are
> specified on the command line or otherwise.  Currently, excludes
> (^<rev>) cancel out that default, so when a command only excludes
> revisions (e.g., 'git log ^origin/master'), it will produce no output.
> 
> With the --sticky-default option, the default becomes more "sticky" and
> is no longer canceled out by excludes.
> 
> This is useful in wrappers that exclude certain revisions: for example,
> a simple alias l='git log --sticky-default ^origin/master' will show the
> revisions between origin/master and HEAD when invoked without arguments,
> and 'l foo' will show the revisions between origin/master and foo.

Your explanation makes sense.

> ---
>  revision.c     | 31 +++++++++++++++++++++++++++----
>  revision.h     |  1 +
>  t/t4202-log.sh |  6 ++++++

We'd probably want an update to Documentation/rev-list-options.txt (I
notice that "--default" is not covered there; that might be worth
fixing, too)>

> +static int has_revisions(struct rev_info *revs)
> +{
> +	struct rev_cmdline_info *info = &revs->cmdline;
> +
> +	return info->nr != 0;
> +}

So this function is going to replace this flag:

> @@ -2401,8 +2423,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;
>  	}

Are we sure that every that hits that "else" is going to trigger
info->nr (and vice versa)?

If I say "--tags", I think we may end up with entries in revs->cmdline,
but would not have set got_rev_arg. That's captured separately in
revs->rev_input_given. But your cancel_default logic:

> @@ -2431,7 +2451,10 @@ 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) {
> +	cancel_default = revs->sticky_default ?
> +			 has_interesting_revisions(revs) :
> +			 has_revisions(revs);
> +	if (revs->def && !revs->rev_input_given && !cancel_default) {

doesn't handle that. So if I do:

  git rev-list --count --sticky-default --default HEAD --not --tags

I should see everything in HEAD that's not tagged. But we don't even
look at cancel_default, because !revs->rev_input_given is not true.

I think you could solve that by making the logic more like:

  if (revs->sticky_default)
	cancel_default = has_interesting_revisions();
  else
	cancel_default = !revs->rev_input_given && !got_rev_arg;

which leaves the non-sticky case exactly as it is today.

> diff --git a/revision.h b/revision.h
> index 2b30ac270..570fa1a6d 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -92,6 +92,7 @@ struct rev_info {
>  
>  	unsigned int early_output;
>  
> +	unsigned int	sticky_default:1;
>  	unsigned int	ignore_missing:1,
>  			ignore_missing_links:1;

Maybe it would make sense to put this next to "const char *def"?

The bitfield would not be as efficient, but I don't think we care about
packing rev_info tightly.

> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 153a50615..9517a65da 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 '--sticky-default ^<rev>' '
> +	git log --pretty="tformat:%s" --sticky-default ^HEAD~2 > actual &&
> +	test_cmp expect actual
> +'

Yuck, t4202 is a mix of older and newer styles. I'm OK with this as-is
because you've matched the surrounding code, but these days I'd probably
write:

  test_expect_success '--sticky-default ^<rev>' '
	{
		echo sixth
		echo fifth
	} >expect &&
	git log --format=%s --sticky-default ^HEAD~2 >actual &&
	test_cmp expect actual
  '

-Peff

Re: [RFC] revision: Add --sticky-default option

[Matthew DeVore]

On Wed, 17 Oct 2018, Jeff King wrote:

>
> Yuck, t4202 is a mix of older and newer styles. I'm OK with this as-is
> because you've matched the surrounding code, but these days I'd probably
> write:
>
>  test_expect_success '--sticky-default ^<rev>' '
> 	{
> 		echo sixth
> 		echo fifth
> 	} >expect &&
> 	git log --format=%s --sticky-default ^HEAD~2 >actual &&
> 	test_cmp expect actual
>  '
>

How about test_write_lines? That is a little more readable to me than
the echos in a subshell. A patch was recently queued with a usage of
that function:

https://public-inbox.org/git/CAMfpvhK4a15gd-PT3W+4YJmpe6c7HyhJE5N_UqOzu8gsYYej4A@mail.gmail.com/T/#m9b5ade1551722938ac97b75af58fec195f246c01

- Matt

Re: [RFC] revision: Add --sticky-default option

[Jeff King]

On Wed, Oct 17, 2018 at 06:24:05AM -0700, Matthew DeVore wrote:

> > Yuck, t4202 is a mix of older and newer styles. I'm OK with this as-is
> > because you've matched the surrounding code, but these days I'd probably
> > write:
> > 
> >  test_expect_success '--sticky-default ^<rev>' '
> > 	{
> > 		echo sixth
> > 		echo fifth
> > 	} >expect &&
> > 	git log --format=%s --sticky-default ^HEAD~2 >actual &&
> > 	test_cmp expect actual
> >  '
> > 
> 
> How about test_write_lines? That is a little more readable to me than
> the echos in a subshell. A patch was recently queued with a usage of
> that function:

Ah, yeah, that would be fine. I was trying to avoid a cat/here-doc combo
since it incurs a process, but test_write_lines is readable and fast.

The main style things I wanted to show are:

  - setting up the expect file should go in the test_expect block

  - no space between ">" and the filename

Those are both present in the surrounding code, but we're slowly
cleaning them up.

-Peff

Re: [RFC] revision: Add --sticky-default option

[Andreas Gruenbacher]

On Wed, 17 Oct 2018 at 11:12, Jeff King <peff@peff.net> wrote:
> On Tue, Oct 16, 2018 at 11:24:38PM +0200, Andreas Gruenbacher wrote:
> > here's a long-overdue update of my proposal from August 29:
> >
> >   [RFC] revision: Don't let ^<rev> cancel out the default <rev>
> >
> > Does this look more acceptable that my first shot?
>
> I think it's going in the right direction.
>
> The name "--sticky-default" did not immediately make clear to me what it
> does. Is there some name that would be more obvious?

It's the best I could think of. Better ideas, anyone?

> > Some commands like 'log' default to HEAD if no other revisions are
> > specified on the command line or otherwise.  Currently, excludes
> > (^<rev>) cancel out that default, so when a command only excludes
> > revisions (e.g., 'git log ^origin/master'), it will produce no output.
> >
> > With the --sticky-default option, the default becomes more "sticky" and
> > is no longer canceled out by excludes.
> >
> > This is useful in wrappers that exclude certain revisions: for example,
> > a simple alias l='git log --sticky-default ^origin/master' will show the
> > revisions between origin/master and HEAD when invoked without arguments,
> > and 'l foo' will show the revisions between origin/master and foo.
>
> Your explanation makes sense.
>
> > ---
> >  revision.c     | 31 +++++++++++++++++++++++++++----
> >  revision.h     |  1 +
> >  t/t4202-log.sh |  6 ++++++
>
> We'd probably want an update to Documentation/rev-list-options.txt (I
> notice that "--default" is not covered there; that might be worth
> fixing, too)>

Ok.

> > +static int has_revisions(struct rev_info *revs)
> > +{
> > +     struct rev_cmdline_info *info = &revs->cmdline;
> > +
> > +     return info->nr != 0;
> > +}
>
> So this function is going to replace this flag:
>
> > @@ -2401,8 +2423,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;
> >       }
>
> Are we sure that every that hits that "else" is going to trigger
> info->nr (and vice versa)?
>
> If I say "--tags", I think we may end up with entries in revs->cmdline,
> but would not have set got_rev_arg. That's captured separately in
> revs->rev_input_given. But your cancel_default logic:
>
> > @@ -2431,7 +2451,10 @@ 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) {
> > +     cancel_default = revs->sticky_default ?
> > +                      has_interesting_revisions(revs) :
> > +                      has_revisions(revs);
> > +     if (revs->def && !revs->rev_input_given && !cancel_default) {
>
> doesn't handle that. So if I do:
>
>   git rev-list --count --sticky-default --default HEAD --not --tags
>
> I should see everything in HEAD that's not tagged. But we don't even
> look at cancel_default, because !revs->rev_input_given is not true.
>
> I think you could solve that by making the logic more like:
>
>   if (revs->sticky_default)
>         cancel_default = has_interesting_revisions();
>   else
>         cancel_default = !revs->rev_input_given && !got_rev_arg;
>
> which leaves the non-sticky case exactly as it is today.

Right, I've reworked that.

> > diff --git a/revision.h b/revision.h
> > index 2b30ac270..570fa1a6d 100644
> > --- a/revision.h
> > +++ b/revision.h
> > @@ -92,6 +92,7 @@ struct rev_info {
> >
> >       unsigned int early_output;
> >
> > +     unsigned int    sticky_default:1;
> >       unsigned int    ignore_missing:1,
> >                       ignore_missing_links:1;
>
> Maybe it would make sense to put this next to "const char *def"?
>
> The bitfield would not be as efficient, but I don't think we care about
> packing rev_info tightly.

Ok.

> > diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> > index 153a50615..9517a65da 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 '--sticky-default ^<rev>' '
> > +     git log --pretty="tformat:%s" --sticky-default ^HEAD~2 > actual &&
> > +     test_cmp expect actual
> > +'
>
> Yuck, t4202 is a mix of older and newer styles. I'm OK with this as-is
> because you've matched the surrounding code, but these days I'd probably
> write:
>
>   test_expect_success '--sticky-default ^<rev>' '
>         {
>                 echo sixth
>                 echo fifth
>         } >expect &&
>         git log --format=%s --sticky-default ^HEAD~2 >actual &&
>         test_cmp expect actual
>   '

I don't really want to get hung up on such details. test_write_lines
doesn't seem bad, either.

Thanks,
Andreas

Re: [RFC] revision: Add --sticky-default option

[Jeff King]

On Wed, Oct 17, 2018 at 03:53:41PM +0200, Andreas Gruenbacher wrote:

> On Wed, 17 Oct 2018 at 11:12, Jeff King <peff@peff.net> wrote:
> > On Tue, Oct 16, 2018 at 11:24:38PM +0200, Andreas Gruenbacher wrote:
> > > here's a long-overdue update of my proposal from August 29:
> > >
> > >   [RFC] revision: Don't let ^<rev> cancel out the default <rev>
> > >
> > > Does this look more acceptable that my first shot?
> >
> > I think it's going in the right direction.
> >
> > The name "--sticky-default" did not immediately make clear to me what it
> > does. Is there some name that would be more obvious?
> 
> It's the best I could think of. Better ideas, anyone?

I'd probably call it something verbose and boring like
--use-default-with-uninteresting or --default-on-negative.
I dunno.

-Peff

Re: [RFC] revision: Add --sticky-default option

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I'd probably call it something verbose and boring like
> --use-default-with-uninteresting or --default-on-negative.
> I dunno.

These two names are improvement, but there needs a hint that the
change we are interested in is to use default even when revs are
given as long as ALL of them are negative ones.  Which in turn means
there is NO positive ones given.  

So perhaps "--use-default-without-any-positive".

Having said that, I have to wonder how serious a breakage we are
going to cause to established users and scripts if we made this
change without any explicit option.  After all, it would be rather
obvious that people will get a history with some commits (or none at
all) when they were expecting no output that the "default behaviour"
has changed.  I also wonder how would scripts take advantage of the
current "defeat --default as soon as we see any rev, even a negative
one"---in short, I am not sure if the theoretical regression this
new "option" is trying to avoid is worth avoiding in the first
place.

Is there a way to say "usually this command has built-in --default=HEAD
behaviour, but I am declining that" already, i.e. 

    $ git log --no-default $REVS

that will result in an empty set if we accept the change proposed
here but make it unconditional?  If so "This and future versions of
Git will honor the --default even when there are other revisions
given on the command line, as long as they are ALL negative ones.
This is a backward incompatibile change, but you can update your
scripts with '--no-default' if you do not like the new behaviour" in
the release notes may be a viable alternative way forward.

If there is no such way in the released versions of Git, then that
would not work, and a strict opt-in like the approach taken by the
proposed patch would become necessary.

Re: [RFC] revision: Add --sticky-default option

[Jeff King]

On Thu, Oct 18, 2018 at 12:23:26PM +0900, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > I'd probably call it something verbose and boring like
> > --use-default-with-uninteresting or --default-on-negative.
> > I dunno.
> 
> These two names are improvement, but there needs a hint that the
> change we are interested in is to use default even when revs are
> given as long as ALL of them are negative ones.  Which in turn means
> there is NO positive ones given.
> 
> So perhaps "--use-default-without-any-positive".

Yeah, though that's getting pretty long. Another way is probably to say
"only" more clearly. Like "--default-on-only-negative" or something.

> Having said that, I have to wonder how serious a breakage we are
> going to cause to established users and scripts if we made this
> change without any explicit option.  After all, it would be rather
> obvious that people will get a history with some commits (or none at
> all) when they were expecting no output that the "default behaviour"
> has changed.  I also wonder how would scripts take advantage of the
> current "defeat --default as soon as we see any rev, even a negative
> one"---in short, I am not sure if the theoretical regression this
> new "option" is trying to avoid is worth avoiding in the first
> place.

Just to play devil's advocate, how about this:

  git log --branches=jk/* --not origin/master

Right now that shows nothing if there are no matching branches. But I
think under the proposed behavior, it would start showing HEAD, which
seems counter-intuitive.

Or are we going to count any positive selector as a positive ref, even
if it matches nothing? I could buy that, though it means that the
command above is subtly different from one or both of:

  branches() {
    git for-each-ref --format='%(refname)' refs/heads/jk/*
  }

  # is --stdin a selector, too?
  branches | git log --stdin --not origin/master

  # here we have no idea that the user did a query and must show HEAD
  git log $(branches) --not origin/master

> Is there a way to say "usually this command has built-in --default=HEAD
> behaviour, but I am declining that" already, i.e. 
> 
>     $ git log --no-default $REVS

I don't think we have that, but regardless of this patch, it seems like
a potentially useful thing. I think we mostly get around it by the fact
that scripts ought to be using "rev-list", and it does not have such a
default that needs to be overridden.

-Peff

Re: [RFC] revision: Add --sticky-default option

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Just to play devil's advocate, how about this:
>
>   git log --branches=jk/* --not origin/master
>
> Right now that shows nothing if there are no matching branches. But I
> think under the proposed behavior, it would start showing HEAD, which
> seems counter-intuitive.
>
> Or are we going to count any positive selector as a positive ref, even
> if it matches nothing? 

That sounds like an intuitive behaviour of the command, but I may
change my mind when I look at other examples.  

When viewing that "--branches=jk/*" example in isolation, yes, these
positive selectors that could produce positive revs should defeat
the --default, especially when it is built-in (like "log").  When
given by the user, I am not sure.  With something like this:

	git rev-list --default=HEAD --branches=jk/* ^master

clearly the user anticipates that jk/* may or may not produce any
positive refs; otherwise there is no point specifying the default.

But anyway...

> I could buy that, though it means that the
> command above is subtly different from one or both of:
>
>   branches() {
>     git for-each-ref --format='%(refname)' refs/heads/jk/*
>   }
>
>   # is --stdin a selector, too?
>   branches | git log --stdin --not origin/master
>
>   # here we have no idea that the user did a query and must show HEAD
>   git log $(branches) --not origin/master

OK, scrap that---just as I predicted a few minutes ago, I now think
that "do we have a positive selector that can produce zero or more
result?" is an ill-defined question X-<.

Thanks for a doze of sanity.

Re: [RFC] revision: Add --sticky-default option

[Andreas Gruenbacher]

On Thu, 18 Oct 2018 at 08:59, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
> > Just to play devil's advocate, how about this:
> >
> >   git log --branches=jk/* --not origin/master
> >
> > Right now that shows nothing if there are no matching branches. But I
> > think under the proposed behavior, it would start showing HEAD, which
> > seems counter-intuitive.
> >
> > Or are we going to count any positive selector as a positive ref, even
> > if it matches nothing?
>
> That sounds like an intuitive behaviour of the command, but I may
> change my mind when I look at other examples.
>
> When viewing that "--branches=jk/*" example in isolation, yes, these
> positive selectors that could produce positive revs should defeat
> the --default, especially when it is built-in (like "log").

I agree, that's the kind of behavior I had in mind. (And I think
that's also what the code implements.)

> When given by the user, I am not sure.  With something like this:
>
>         git rev-list --default=HEAD --branches=jk/* ^master
>
> clearly the user anticipates that jk/* may or may not produce any
> positive refs; otherwise there is no point specifying the default.

With positive selectors, it is meaningful if the selectors match
nothing. So in the above example, if jk/* matches nothing, I would
really expect no output, and the default should not be applied.
So we should just document that --default=<rev> only sets the default
in case the default is used.

> But anyway...
>
> > I could buy that, though it means that the
> > command above is subtly different from one or both of:
> >
> >   branches() {
> >     git for-each-ref --format='%(refname)' refs/heads/jk/*
> >   }
> >
> >   # is --stdin a selector, too?
> >   branches | git log --stdin --not origin/master

Yes, it's a positive selector (since --not doesn't apply to --stdin).

> >   # here we have no idea that the user did a query and must show HEAD
> >   git log $(branches) --not origin/master

In this case, 'git log' is more used like git rev-list which doesn't
default to HEAD. The 'git log --no-default' would neatly restore
sanity here.

> OK, scrap that---just as I predicted a few minutes ago, I now think
> that "do we have a positive selector that can produce zero or more
> result?" is an ill-defined question X-<.
>
> Thanks for a doze of sanity.

Andreas

Re: [RFC] revision: Add --sticky-default option

[Junio C Hamano]

Andreas Gruenbacher <agruenba@redhat.com> writes:

>> >   # is --stdin a selector, too?
>> >   branches | git log --stdin --not origin/master
>
> Yes, it's a positive selector (since --not doesn't apply to --stdin).

But you should be able to do

	printf "%s\n" ^maint master | git rev-list --stdin

Replace the second one with ^master and now you have nothing but
negative.

So, no, the line is much blurrier than you would think.

Re: [RFC] revision: Add --sticky-default option

[Andreas Gruenbacher]

On Thu, 18 Oct 2018 at 05:23, Junio C Hamano <gitster@pobox.com> wrote:
> Jeff King <peff@peff.net> writes:
>
> > I'd probably call it something verbose and boring like
> > --use-default-with-uninteresting or --default-on-negative.
> > I dunno.
>
> These two names are improvement, but there needs a hint that the
> change we are interested in is to use default even when revs are
> given as long as ALL of them are negative ones.  Which in turn means
> there is NO positive ones given.
>
> So perhaps "--use-default-without-any-positive".
>
> Having said that, I have to wonder how serious a breakage we are
> going to cause to established users and scripts if we made this
> change without any explicit option.  After all, it would be rather
> obvious that people will get a history with some commits (or none at
> all) when they were expecting no output that the "default behaviour"
> has changed.  I also wonder how would scripts take advantage of the
> current "defeat --default as soon as we see any rev, even a negative
> one"---in short, I am not sure if the theoretical regression this
> new "option" is trying to avoid is worth avoiding in the first
> place.
>
> Is there a way to say "usually this command has built-in --default=HEAD
> behaviour, but I am declining that" already, i.e.
>
>     $ git log --no-default $REVS
>
> that will result in an empty set if we accept the change proposed
> here but make it unconditional?  If so "This and future versions of
> Git will honor the --default even when there are other revisions
> given on the command line, as long as they are ALL negative ones.
> This is a backward incompatibile change, but you can update your
> scripts with '--no-default' if you do not like the new behaviour" in
> the release notes may be a viable alternative way forward.

That would certainly work for me.

Andreas

> If there is no such way in the released versions of Git, then that
> would not work, and a strict opt-in like the approach taken by the
> proposed patch would become necessary.