Re: [PATCH] Clear --exclude list after 'git rev-parse --all'

2018-10-24 Thread Andreas Gruenbacher
On Wed, 24 Oct 2018 at 11:24, Junio C Hamano  wrote:
> Andreas Gruenbacher  writes:
> >> All other glob options do show_reference with for_each_ref_in() and
> >> then calls clear_ref_exclusion(), and logically the patch makes
> >> sense.
> >>
> >> What is the "problem" this patch fixes, though?  Is it easy to add a
> >> new test to t/6018-rev-list-glob.sh to demonstrate that "--glob" (or
> >> whatever that clears exclusion list without this patch) works
> >> correctly but "--all" misbehaves without this change?
> >
> > The test suite doesn't cover clearing the exclusion list for any of
> > those rev-parse options and I also didn't write such a test case. I
> > ran into this inconsistency during code review.
>
> That is why I asked what "problem" this patch fixes.  Without
> answering that question, it is unclear if the patch is completing
> missing coverage for "--all", or it is cargo culting an useless
> clearing done for "--glob" and friends to code for "--all" that did
> not do the same useless clearing.

This is how the --exclude option is described:

   --exclude=
   Do not include refs matching  that the next
--all, --branches,
   --tags, --remotes, or --glob would otherwise consider.
Repetitions of this
   option accumulate exclusion patterns up to the next --all,
--branches, --tags,
   --remotes, or --glob option (other options or arguments do not clear
   accumulated patterns).

I'm assuming that some thought has gone into making the options behave
in this particular way. The implementation in revision.c follows this
pattern, but the implementation in builtin/rev-parse.c only almost
does.

Andreas


Re: [PATCH] Clear --exclude list after 'git rev-parse --all'

2018-10-24 Thread Andreas Gruenbacher
On Wed, 24 Oct 2018 at 06:35, Junio C Hamano  wrote:
> Andreas Gruenbacher  writes:
>
> > Commit [1] added the --exclude option to revision.c.  The --all,
> > --branches, --tags, --remotes, and --glob options clear the exclude
> > list.  Shortly therafter, commit [2] added the same to 'git rev-parse',
> > but without clearing the exclude list for the --all option.  Fix that.
> >
> > [1] e7b432c52 ("revision: introduce --exclude= to tame wildcards", 
> > 2013-08-30)
> > [2] 9dc01bf06 ("rev-parse: introduce --exclude= to tame wildcards", 
> > 2013-11-01)
> >
> > Signed-off-by: Andreas Gruenbacher 
> > ---
> >  builtin/rev-parse.c | 1 +
> >  1 file changed, 1 insertion(+)
>
> All other glob options do show_reference with for_each_ref_in() and
> then calls clear_ref_exclusion(), and logically the patch makes
> sense.
>
> What is the "problem" this patch fixes, though?  Is it easy to add a
> new test to t/6018-rev-list-glob.sh to demonstrate that "--glob" (or
> whatever that clears exclusion list without this patch) works
> correctly but "--all" misbehaves without this change?

The test suite doesn't cover clearing the exclusion list for any of
those rev-parse options and I also didn't write such a test case. I
ran into this inconsistency during code review.

Andreas


[PATCH] Clear --exclude list after 'git rev-parse --all'

2018-10-23 Thread Andreas Gruenbacher
Commit [1] added the --exclude option to revision.c.  The --all,
--branches, --tags, --remotes, and --glob options clear the exclude
list.  Shortly therafter, commit [2] added the same to 'git rev-parse',
but without clearing the exclude list for the --all option.  Fix that.

[1] e7b432c52 ("revision: introduce --exclude= to tame wildcards", 
2013-08-30)
[2] 9dc01bf06 ("rev-parse: introduce --exclude= to tame wildcards", 
2013-11-01)

Signed-off-by: Andreas Gruenbacher 
---
 builtin/rev-parse.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 0f09bbbf6..c71e3b104 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -764,6 +764,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
}
if (!strcmp(arg, "--all")) {
for_each_ref(show_reference, NULL);
+   clear_ref_exclusion(&ref_excludes);
continue;
}
if (skip_prefix(arg, "--disambiguate=", &arg)) {
-- 
2.17.2



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

2018-10-18 Thread Andreas Gruenbacher
On Thu, 18 Oct 2018 at 08:53, Jeff King  wrote:
> On Wed, Oct 17, 2018 at 03:49:47PM +0200, Andreas Gruenbacher wrote:
> > @@ -2431,7 +2446,11 @@ 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->sticky_default)
> > + cancel_default = has_interesting_revisions();
> > + else
> > + cancel_default = got_rev_arg;
> > + if (revs->def && !revs->rev_input_given && !cancel_default) {
>
> How do you want to handle "maybe has a ref" options like --stdin,
> --tags, etc? Those set revs->rev_input_given.
>
> With the code you have here, rev_input_given overrides any
> sticky_default decision, and you cannot do this:
>
>   git log --not --remotes=origin
>
> and get the default, because even though you have only UNINTERESTING
> commits, rev_input_given is true.
>
> If you move rev_input_given to the cancel_default line above the final
> conditional and turn that conditional into:
>
>   if (revs->def && !cancel_default)
>
> then it works.

Yes, this still needs fixing, I'm just not sure yet if this is the right way.

Thanks,
Andreas


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

2018-10-18 Thread Andreas Gruenbacher
On Thu, 18 Oct 2018 at 05:23, Junio C Hamano  wrote:
> Jeff King  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.


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

2018-10-18 Thread Andreas Gruenbacher
On Thu, 18 Oct 2018 at 08:59, Junio C Hamano  wrote:
> Jeff King  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= 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

2018-10-17 Thread Andreas Gruenbacher
On Wed, 17 Oct 2018 at 11:12, Jeff King  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 ^ cancel out the default 
> >
> > 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
> > (^) 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 intsticky_default:1;
> >   unsigned intignore_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_e

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

2018-10-17 Thread Andreas Gruenbacher
Some commands like 'log' default to HEAD if no other revisions are
specified on the command line or otherwise.  Currently, excludes
(^) 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 
---
 Documentation/rev-list-options.txt | 10 ++
 revision.c | 21 -
 revision.h |  1 +
 t/t4202-log.sh |  6 ++
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 7b273635d..46fab2b42 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -210,6 +210,16 @@ endif::git-rev-list[]
seen, stop reading commits and start reading paths to limit the
result.
 
+--default=::
+   Instead of `HEAD`, use '' when no revisions are specified.
+
+--sticky-default:
+   By default, excludes ('^') override the default revision (i.e.
+   `HEAD` or the revision specified with `--default`).  This option causes
+   excludes to no longer override the default revision, so commands like
+   `git log --sticky-default ^origin/master` will continue to operate on
+   the default revision as long as no other revisions are specified.
+
 ifdef::git-rev-list[]
 --quiet::
Don't print anything to standard output.  This form
diff --git a/revision.c b/revision.c
index e18bd530e..e61f28b92 100644
--- a/revision.c
+++ b/revision.c
@@ -1159,6 +1159,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,
@@ -2132,6 +2144,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")) {
@@ -2320,6 +2334,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, got_rev_arg = 0, revarg_opt;
+   int cancel_default;
struct argv_array prune_data = ARGV_ARRAY_INIT;
const char *submodule = NULL;
 
@@ -2431,7 +2446,11 @@ 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->sticky_default)
+   cancel_default = has_interesting_revisions();
+   else
+   cancel_default = got_rev_arg;
+   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..6498ba263 100644
--- a/revision.h
+++ b/revision.h
@@ -73,6 +73,7 @@ struct rev_info {
/* Basic information */
const char *prefix;
const char *def;
+   unsigned int sticky_default:1;
struct pathspec prune_data;
 
/*
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 153a50615..5ac93f5ec 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -213,6 +213,12 @@ test_expect_success 'git show  leaves list of 
commits as given' '
test_cmp expect actual
 '
 
+test_expect_success '--sticky-default ^' '
+   test_write_lines sixth fifth > expect &&
+   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



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

2018-10-16 Thread Andreas Gruenbacher
Hi,

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

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

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
(^) 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 
---
 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 intsticky_default:1;
unsigned intignore_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  leaves list of 
commits as given' '
test_cmp expect actual
 '
 
+printf "sixth\nfifth\n" > expect
+test_expect_success '--sticky-default ^' '
+   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



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

2018-08-29 Thread Andreas Gruenbacher
Some commands like 'log' default to HEAD if no other revisions are
specified on the command line or otherwise.  Unfortunately, excludes
(^) 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 
---
 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  leaves list of 
commits as given' '
test_cmp expect actual
 '
 
+printf "sixth\nfifth\n" > expect
+test_expect_success '^' '
+   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: [PATCH] Inconsistency between git log and git rev-parse for ^HEAD^@

2017-03-18 Thread Andreas Gruenbacher
On Sat, Mar 18, 2017 at 9:18 PM, Junio C Hamano  wrote:
> Andreas Gruenbacher  writes:
>
>> Hello,
>>
>> the log and rev-parse commands both support the rev^@ syntax which stands for
>> all parents of rev.  The log command also supports ^rev^@ to exclude all of 
>> the
>> parents of rev, but rev-parse does not.  Should this be fixed?
>>
>> If so, the following patch would be a start.
>
> Hmph, would ^A..B and ^A...B also be turned into B..A and B...A in a
> similar way?  I think the latter would not make much sense but ^A..B
> might.

The previous patch supports neither. I agree about ^A...B, and I don't
think supporting ^A..B is relevant.

> In any case, accepting ^rev^@ may make sense nevertheless.

Andreas


[PATCH] Inconsistency between git log and git rev-parse for ^HEAD^@

2017-03-18 Thread Andreas Gruenbacher
Hello,

the log and rev-parse commands both support the rev^@ syntax which stands for
all parents of rev.  The log command also supports ^rev^@ to exclude all of the
parents of rev, but rev-parse does not.  Should this be fixed?

If so, the following patch would be a start.

Thanks,
Andreas

--

rev-parse: Add support for ^rev^@

Add support for the ^rev^@ syntax to exclude all of the parents of rev.  This
syntax is already supported by git log.

Signed-off-by: Andreas Gruenbacher 
---
 builtin/rev-parse.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 2549643..ab84c49 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -293,7 +293,7 @@ static int try_difference(const char *arg)
return 0;
 }
 
-static int try_parent_shorthands(const char *arg)
+static int try_parent_shorthands(int type, const char *arg)
 {
char *dotdot;
unsigned char sha1[20];
@@ -339,7 +339,7 @@ static int try_parent_shorthands(const char *arg)
}
 
if (include_rev)
-   show_rev(NORMAL, sha1, arg);
+   show_rev(type, sha1, arg);
for (parents = commit->parents, parent_number = 1;
 parents;
 parents = parents->next, parent_number++) {
@@ -350,7 +350,7 @@ static int try_parent_shorthands(const char *arg)
 
if (symbolic)
name = xstrfmt("%s^%d", arg, parent_number);
-   show_rev(include_parents ? NORMAL : REVERSED,
+   show_rev(include_parents ? type : !type,
 parents->item->object.oid.hash, name);
free(name);
}
@@ -896,14 +896,14 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
/* Not a flag argument */
if (try_difference(arg))
continue;
-   if (try_parent_shorthands(arg))
-   continue;
name = arg;
type = NORMAL;
if (*arg == '^') {
name++;
type = REVERSED;
}
+   if (try_parent_shorthands(type, name))
+   continue;
if (!get_sha1_with_context(name, flags, sha1, &unused)) {
if (verify)
revs_count++;
-- 
2.7.4



Re: patch-2.7.3 no longer applies relative symbolic link patches

2015-01-27 Thread Andreas Gruenbacher
On Mon, 26 Jan 2015 12:44:33 -0800, Linus Torvalds wrote:
> I've considered that for a while already, because "patch" _does_ kind of
> understand them these days, although I think it gets the cross-rename
> case wrong because it fundamentally works on a file-by-file basis.

Patch handles cross-renames correctly nowadays; if not, it's a bug.

That's not enough to solve the symlink problem unfortunately.

Andreas

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html