Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Junio C Hamano]

Rafael Ascensão  writes:

> I agree that describe should receive the "normalize" treatment. However,
> and following the same reasoning, why should describe users adopt the
> rules imposed by --glob? I could argue they're also used to the way it
> works now.
>
> That being said, the suggestion I mentioned earlier would allow to keep
> both current behaviors consistent at the expense of the extra call to
> refs.c::ref_exists().

In any case, updating the "describe" for consistency is something we
can and should leave for later, to be done as a separate topic.

While I agree with you that the consistent behaviour between
commands is desirable, and also I agree with you that given a
pattern $X that does not have any glob char, trying to match $X when
a ref whose name exactly is $X exists and trying to match $X/*
otherwise would give us a consistent semantics without hurting any
existing uses, I do not think you need to pay any extra expense of
calling ref_exists() at all to achieve that.

That is because when $X exists, you already know $X/otherthing does
not exist.  And when $X does not exist, $X/otherthing might.  So a
naive implementation would be just to add two patterns $X and $X/*
to the filter list and be done with it.  If you exactly have
refs/heads/master, even with the naive logic may throw both
refs/heads/master and refs/heads/master/* to the filter list,
nothing will match the latter to contaminate your result (and vice
versa).

A bit more clever implementation "just throw in two items" would go
like this.  It is not all that involved:

 - In load_ref_decorations(), before running add_ref_decoration for
   each ref and head ref, iterate over the elements in the refname
   filter list.  For each element:

   - if item->string has a trailing '/', trim that.

   - store NULL in the item->util field for item whose string field
 has a glob char.

   - store something non-NULL (e.g. item->string) for item whose
 string field does not have a glob char.

 - In add_ref_decoration(), where your previous round iterates over
   filter->{include,exclude}, get rid of normalize_glob_ref() and
   use of real_pattern.  Instead do something like:

matched = 0;
if (item->util == NULL) {
if (!wildmatch(item->string, refname, 0))
matched = 1;
} else {
const char *rest;
if (skip_prefix(refname, item->string, ) &&
(!*rest || *rest == '/'))
matched = 1;
}
if (matched)
...

   Of course, you would probably want to encapsulate the logic to
   set matched = 1/0 in a helper function, e.g.

static int match_ref_pattern(const char *refname,
 const struct string_list_item *item) {
int matched = 0;
... do either wildmatch or head match with tail validation
... depending on the item->util's NULLness (see above)
return matched;
}

   and call that from the two loops for exclude and include list.

Hmm?

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Rafael Ascensão]

On 07/11/17 00:18, Junio C Hamano wrote:
> Jacob Keller  writes:
> 
> I would have to say that the describe's one is wrong if it does not
> match what for_each_glob_ref() does for the log family of commands'
> "--branches=" etc.  describe.c::get_name() uses positive
> and negative patterns, just like log-tree.c::add_ref_decoration()
> would with the patch we are discussing, so perhaps the items in
> these lists should get the same "normalize" treatment the patch 1/2
> of this series brings in to make things consistent?
> 

I agree that describe should receive the "normalize" treatment. However,
and following the same reasoning, why should describe users adopt the
rules imposed by --glob? I could argue they're also used to the way it
works now.

That being said, the suggestion I mentioned earlier would allow to keep
both current behaviors consistent at the expense of the extra call to
refs.c::ref_exists().

+if (!has_glob_specials(pattern) && !ref_exists(normalized_pattern->buf)) {
+/* Append implied '/' '*' if not present. */
+strbuf_complete(normalized_pattern, '/');
+/* No need to check for '*', there is none. */
+strbuf_addch(normalized_pattern, '*');
+}

But I don't have enough expertise to decide if this consistency is worth 
the extra call to refs.c::ref_exists() or if there are other side-effects
I am not considering.

>> That being said, if we think the extra glob would not cause
>> problems and generally do what people mean... I guess consistent
>> with --glob would be good... But it's definitely not what I'd
>> expect at first glance.

My position is that consistency is good, but the "first glance
expectation" is definitely something important we should take into
consideration.

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Junio C Hamano]

Jacob Keller  writes:

> On November 3, 2017 8:49:15 PM PDT, Junio C Hamano  wrote:
>>Rafael Ascensão  writes:
>>
>>Why should this be a special case that burdens users to remember one
>>more rule?  Wouldn't users find "--decorate-refs=refs/tags" useful
>>and it woulld be shorter and nicer than having to say "refs/tags/*"?
>
> Actually, I would expect these to behave more like git describes
> match and exclude which don't have an extra /*. It seems natural
> to me that glob would always add an extra glob, but.. I don't
> recall if match and exlude do so.

I would have to say that the describe's one is wrong if it does not
match what for_each_glob_ref() does for the log family of commands'
"--branches=" etc.  describe.c::get_name() uses positive
and negative patterns, just like log-tree.c::add_ref_decoration()
would with the patch we are discussing, so perhaps the items in
these lists should get the same "normalize" treatment the patch 1/2
of this series brings in to make things consistent?

> That being said, if we think the extra glob would not cause
> problems and generally do what people mean... I guess consistent
> with --glob would be good... But it's definitely not what I'd
> expect at first glance.

FWIW, what describe --match/--exclude do is not what I'd have
expected ;-)  In any case, we spotted an existing inconsistency that
we would want to resolve (the resolution could be "leave it as-is";
I do not think we have thought this through enough yet), which is
good.

Thanks.

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Jacob Keller]

On November 3, 2017 8:49:15 PM PDT, Junio C Hamano  wrote:
>Rafael Ascensão  writes:
>
>Why should this be a special case that burdens users to remember one
>more rule?  Wouldn't users find "--decorate-refs=refs/tags" useful
>and it woulld be shorter and nicer than having to say "refs/tags/*"?
>

Actually, I would expect these to behave more like git describes match and 
exclude which don't have an extra /*. It seems natural to me that glob would 
always add an extra glob, but.. I don't recall if match and exlude do so.

That being said, if we think the extra glob would not cause problems and 
generally do what people mean... I guess consistent with --glob would be 
good... But it's definitely not what I'd expect at first glance.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Michael Haggerty]

On 11/05/2017 07:17 AM, Junio C Hamano wrote:
> Junio C Hamano  writes:
>> Rafael Ascensão  writes:
>> ...
>>> Because changing the default behavior of that function has
>>> implications on multiple commands which I think shouldn't change. But
>>> at the same time, would be nice to have the logic that deals with
>>> glob-ref patterns all in one place.
>>>
>>> What's the sane way to do this?
>>
>> Learn to type "--decorate-refs="refs/heads/[m]aster", and not twewak
>> the code at all, perhaps.  The users of existing "with no globbing,
>> /* is appended" interface are already used to that way and they do
>> not have to learn a new and inconsistent interface.
>>
>> After all, "I only want to see 'git log' output with 'master'
>> decorated" (i.e. not specifying "this class of refs I can glob by
>> using the naming convention I am using" and instead enumerating the
>> ones you care about) does not sound like a sensible thing people
>> often want to do, so making it follow the other codepath so that
>> people can say "refs/tags" to get "refs/tags/*", while still allowing
>> such a rare but specific and exact one possible, may not sound too
>> bad to me.
> 
> Having said all that, I can imagine another way out might be to
> change the behaviour of this "normalize" thing to add two patterns,
> the original pattern in addition to the original pattern plus "/*",
> when it sees a pattern without any glob.  Many users who relied on
> the current behaviour fed "refs/tags" knowing that it will match
> everything under "refs/tags" i.e. "refs/tags/*", and they cannot
> have a ref that is exactly "refs/tags", so adding the original
> pattern without an extra trailing "/*" would not hurt them.  And
> this will allow you to say "refs/heads/master" when you know you
> want that exact ref, and in such a repository where that original
> pattern without trailing "/*" would be useful, because you cannot
> have "refs/heads/master/one" at the same time, having an extra
> pattern that is the original plus "/*" would not hurt you, either.
> 
> This however needs a bit of thought to see if there are corner cases
> that may result in unexpected and unwanted fallout, and something I
> am reluctant to declare unilaterally that it is a better way to go.

There's some glob-matching code (somewhere? I don't know if it's allowed
everywhere) that allows "**" to mean "zero or one path components. If
"refs/tags" were massaged to be "refs/tags/**", then it would match not only

refs/tags
refs/tags/foo

but also

refs/tags/foo/bar

, which is probably another thing that the user would expect to see.

There's at least some precedent for this kind of expansion: `git
for-each-ref refs/remotes` lists *all* references under that prefix,
even if they have multiple levels.

Michael

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Junio C Hamano]

Rafael Ascensão  writes:

> Would checking the output of ref_exists() make sense here?
> By that I mean, only add a trailing '/*' if the ref doesn't exist.

I do not think it would hurt, but it is not immediately obvious if
the benefit of doing so outweighs the cost of having to make an
extra call to ref_exists().

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Rafael Ascensão]

Would checking the output of ref_exists() make sense here?
By that I mean, only add a trailing '/*' if the ref doesn't exist.

Unless I am missing something obvious this would allow us to keep both
shortcuts and exact patterns.

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Junio C Hamano]

Junio C Hamano  writes:
> Rafael Ascensão  writes:
> ...
>> Because changing the default behavior of that function has
>> implications on multiple commands which I think shouldn't change. But
>> at the same time, would be nice to have the logic that deals with
>> glob-ref patterns all in one place.
>>
>> What's the sane way to do this?
>
> Learn to type "--decorate-refs="refs/heads/[m]aster", and not twewak
> the code at all, perhaps.  The users of existing "with no globbing,
> /* is appended" interface are already used to that way and they do
> not have to learn a new and inconsistent interface.
>
> After all, "I only want to see 'git log' output with 'master'
> decorated" (i.e. not specifying "this class of refs I can glob by
> using the naming convention I am using" and instead enumerating the
> ones you care about) does not sound like a sensible thing people
> often want to do, so making it follow the other codepath so that
> people can say "refs/tags" to get "refs/tags/*", while still allowing
> such a rare but specific and exact one possible, may not sound too
> bad to me.

Having said all that, I can imagine another way out might be to
change the behaviour of this "normalize" thing to add two patterns,
the original pattern in addition to the original pattern plus "/*",
when it sees a pattern without any glob.  Many users who relied on
the current behaviour fed "refs/tags" knowing that it will match
everything under "refs/tags" i.e. "refs/tags/*", and they cannot
have a ref that is exactly "refs/tags", so adding the original
pattern without an extra trailing "/*" would not hurt them.  And
this will allow you to say "refs/heads/master" when you know you
want that exact ref, and in such a repository where that original
pattern without trailing "/*" would be useful, because you cannot
have "refs/heads/master/one" at the same time, having an extra
pattern that is the original plus "/*" would not hurt you, either.

This however needs a bit of thought to see if there are corner cases
that may result in unexpected and unwanted fallout, and something I
am reluctant to declare unilaterally that it is a better way to go.

Thoughts?

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Junio C Hamano]

Rafael Ascensão  writes:

>>> The pattern follows similar rules as `--glob` except it doesn't assume a
>>> trailing '/*' if glob characters are missing.
>>
>> Why should this be a special case that burdens users to remember one
>> more rule?  Wouldn't users find "--decorate-refs=refs/tags" useful
>> and it woulld be shorter and nicer than having to say "refs/tags/*"?
>
> I wanted to allow exact patterns like:
> "--decorate-refs=refs/heads/master" and for that I disabled the flag
> that adds the trailing '/*' if no globs are found. As a side effect, I
> lost the shortcut.
>
> Is adding a yet another flag that appends '/*' only if the pattern
> equals "refs/{heads,remotes,tags}" a good idea?

No.

> Because changing the default behavior of that function has
> implications on multiple commands which I think shouldn't change. But
> at the same time, would be nice to have the logic that deals with
> glob-ref patterns all in one place.
>
> What's the sane way to do this?

Learn to type "--decorate-refs="refs/heads/[m]aster", and not twewak
the code at all, perhaps.  The users of existing "with no globbing,
/* is appended" interface are already used to that way and they do
not have to learn a new and inconsistent interface.

After all, "I only want to see 'git log' output with 'master'
decorated" (i.e. not specifying "this class of refs I can glob by
using the naming convention I am using" and instead enumerating the
ones you care about) does not sound like a sensible thing people
often want to do, so making it follow the other codepath so that
people can say "refs/tags" to get "refs/tags/*", while still allowing
such a rare but specific and exact one possible, may not sound too
bad to me.

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Rafael Ascensão]

On 04/11/17 03:49, Junio C Hamano wrote:

Rafael Ascensão  writes:


Using `--exclude=` can help mitigate that verboseness by
removing unnecessary 'branches' from the output. However, if the tip of
an excluded ref points to an ancestor of a non-excluded ref, git will
decorate it regardless.


Is this even relevant?  I think the above would only serve to
confuse the readers.  --exclude, --branches, etc. are ways to
specify what starting points "git log" history traversal should
begin and has nothing to do with what set of refs are to be used to
decorate the commits that are shown.  But the paragraph makes
readers wonder if it might have any effect in some circumstances.


With `--decorate-refs=`, only refs that match  are
decorated while `--decorate-refs-exclude=` allows to do the
reverse, remove ref decorations that match 


And "Only refs that match ... are decorated" is also confusing.  The
thing is, refs are never decorated, they are used for decorating
commits in the output from "git log".  For example, if you have 


---A---B---C---D

and B is at the tip of the 'master' branch, the output from "git log
D" would decorate B with 'master', even if you do not say 'master'
on the command line as the commit to start the traversal from. >
Perhaps drop the irrelevant paragraph about "--exclude" and write
something like this instead?

When "--decorate-refs=" is given, only the refs
that match the pattern is used in decoration.  The refs that
match the pattern, when "--decorate-refs-exclude="
is given, are never used in decoration.



What you explained was the reason I mentioned that. Because some users 
were wrongfully trying to remove decorations by trying to exclude the 
starting points. But I agree this adds little value and can generate 
further confusion. I will remove that section.



Both can be used together but --decorate-refs-exclude patterns have
precedence over --decorate-refs patterns.


A reasonable and an easy-to-explain way to mix zero or more positive
and zero or more negagive patterns that follows the convention used
elsewhere in the system (e.g. how negative pathspecs work) is

  (1) if there is no positive pattern given, pretend as if an
  inclusive default positive pattern was given;

  (2) for each candidate, reject it if it matches no positive
  pattern, or if it matches any one of negative patterns.

For pathspecs, we use "everything" as the inclusive default positive
pattern, I think, and for the set of refs used for decoration, a
reasonable choice would also be to use "everything", which matches
the current behaviour.



That's a nice explanation that fits the current "--decorate-refs" behavior.


The pattern follows similar rules as `--glob` except it doesn't assume a
trailing '/*' if glob characters are missing.


Why should this be a special case that burdens users to remember one
more rule?  Wouldn't users find "--decorate-refs=refs/tags" useful
and it woulld be shorter and nicer than having to say "refs/tags/*"?



I wanted to allow exact patterns like:
"--decorate-refs=refs/heads/master" and for that I disabled the flag 
that adds the trailing '/*' if no globs are found. As a side effect, I 
lost the shortcut.


Is adding a yet another flag that appends '/*' only if the pattern 
equals "refs/{heads,remotes,tags}" a good idea?


Because changing the default behavior of that function has implications 
on multiple commands which I think shouldn't change. But at the same 
time, would be nice to have the logic that deals with glob-ref patterns 
all in one place.


What's the sane way to do this?


diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 32246fdb0..314417d89 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -38,6 +38,18 @@ OPTIONS
are shown as if 'short' were given, otherwise no ref names are
shown. The default option is 'short'.
  
+--decorate-refs=::

+   Only print ref names that match the specified pattern. Uses the same
+   rules as `git rev-list --glob` except it doesn't assume a trailing a
+   trailing '/{asterisk}' if pattern lacks '?', '{asterisk}', or '['.
+   `--decorate-refs-exlclude` has precedence.
+
+--decorate-refs-exclude=::
+   Do not print ref names that match the specified pattern. Uses the same
+   rules as `git rev-list --glob` except it doesn't assume a trailing a
+   trailing '/{asterisk}' if pattern lacks '?', '{asterisk}', or '['.
+   Has precedence over `--decorate-refs`.



These two may be technically correct, but I wonder if we can make it
easier to understand (I found "precedence" bit hard to follow, as in
my mind, these are ANDed conditions and between (A & ~B), there is
no "precedence").  Also we'd want to clarify what happens when only
"--decorate-refs-exclude"s are given, which in turn necessitates us
to describe what happens when only "--decorate-refs"s are given.


I 

Re: [PATCH v1 2/2] log: add option to choose which refs to decorate

[Junio C Hamano]

Rafael Ascensão  writes:

> When `log --decorate` is used, git will decorate commits with all
> available refs. While in most cases this the desired effect, under some
> conditions it can lead to excessively verbose output.

Correct.

> Using `--exclude=` can help mitigate that verboseness by
> removing unnecessary 'branches' from the output. However, if the tip of
> an excluded ref points to an ancestor of a non-excluded ref, git will
> decorate it regardless.

Is this even relevant?  I think the above would only serve to
confuse the readers.  --exclude, --branches, etc. are ways to
specify what starting points "git log" history traversal should
begin and has nothing to do with what set of refs are to be used to
decorate the commits that are shown.  But the paragraph makes
readers wonder if it might have any effect in some circumstances.

> With `--decorate-refs=`, only refs that match  are
> decorated while `--decorate-refs-exclude=` allows to do the
> reverse, remove ref decorations that match 

And "Only refs that match ... are decorated" is also confusing.  The
thing is, refs are never decorated, they are used for decorating
commits in the output from "git log".  For example, if you have 

---A---B---C---D

and B is at the tip of the 'master' branch, the output from "git log
D" would decorate B with 'master', even if you do not say 'master'
on the command line as the commit to start the traversal from.

Perhaps drop the irrelevant paragraph about "--exclude" and write
something like this instead?

When "--decorate-refs=" is given, only the refs
that match the pattern is used in decoration.  The refs that
match the pattern, when "--decorate-refs-exclude="
is given, are never used in decoration.

> Both can be used together but --decorate-refs-exclude patterns have
> precedence over --decorate-refs patterns.

A reasonable and an easy-to-explain way to mix zero or more positive
and zero or more negagive patterns that follows the convention used
elsewhere in the system (e.g. how negative pathspecs work) is

 (1) if there is no positive pattern given, pretend as if an
 inclusive default positive pattern was given;

 (2) for each candidate, reject it if it matches no positive
 pattern, or if it matches any one of negative patterns.

For pathspecs, we use "everything" as the inclusive default positive
pattern, I think, and for the set of refs used for decoration, a
reasonable choice would also be to use "everything", which matches
the current behaviour.

> The pattern follows similar rules as `--glob` except it doesn't assume a
> trailing '/*' if glob characters are missing.

Why should this be a special case that burdens users to remember one
more rule?  Wouldn't users find "--decorate-refs=refs/tags" useful
and it woulld be shorter and nicer than having to say "refs/tags/*"?

> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 32246fdb0..314417d89 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -38,6 +38,18 @@ OPTIONS
>   are shown as if 'short' were given, otherwise no ref names are
>   shown. The default option is 'short'.
>  
> +--decorate-refs=::
> + Only print ref names that match the specified pattern. Uses the same
> + rules as `git rev-list --glob` except it doesn't assume a trailing a
> + trailing '/{asterisk}' if pattern lacks '?', '{asterisk}', or '['.
> + `--decorate-refs-exlclude` has precedence.
> +
> +--decorate-refs-exclude=::
> + Do not print ref names that match the specified pattern. Uses the same
> + rules as `git rev-list --glob` except it doesn't assume a trailing a
> + trailing '/{asterisk}' if pattern lacks '?', '{asterisk}', or '['.
> + Has precedence over `--decorate-refs`.

These two may be technically correct, but I wonder if we can make it
easier to understand (I found "precedence" bit hard to follow, as in
my mind, these are ANDed conditions and between (A & ~B), there is
no "precedence").  Also we'd want to clarify what happens when only
"--decorate-refs-exclude"s are given, which in turn necessitates us
to describe what happens when only "--decorate-refs"s are given.

> diff --git a/log-tree.c b/log-tree.c
> index cea056234..8efc7ac3d 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -94,9 +94,33 @@ static int add_ref_decoration(const char *refname, const 
> struct object_id *oid,
>  {
>   struct object *obj;
>   enum decoration_type type = DECORATION_NONE;
> + struct ref_include_exclude_list *filter = (struct 
> ref_include_exclude_list *)cb_data;
> + struct string_list_item *item;
> + struct strbuf real_pattern = STRBUF_INIT;
> +
> + if(filter && filter->exclude->nr > 0) {

Have SP before '('.

> + /* if current ref is on the exclude list skip */
> + for_each_string_list_item(item, filter->exclude) {
> + strbuf_reset(_pattern);
> +   

[PATCH v1 2/2] log: add option to choose which refs to decorate

[Rafael Ascensão]

When `log --decorate` is used, git will decorate commits with all
available refs. While in most cases this the desired effect, under some
conditions it can lead to excessively verbose output.

Using `--exclude=` can help mitigate that verboseness by
removing unnecessary 'branches' from the output. However, if the tip of
an excluded ref points to an ancestor of a non-excluded ref, git will
decorate it regardless.

With `--decorate-refs=`, only refs that match  are
decorated while `--decorate-refs-exclude=` allows to do the
reverse, remove ref decorations that match 

Both can be used together but --decorate-refs-exclude patterns have
precedence over --decorate-refs patterns.

The pattern follows similar rules as `--glob` except it doesn't assume a
trailing '/*' if glob characters are missing.

Both `--decorate-refs` and `--decorate-refs-exclude` can be used
multiple times.

Signed-off-by: Kevin Daudt 
Signed-off-by: Rafael Ascensão 
---
 Documentation/git-log.txt |  12 ++
 builtin/log.c |  10 -
 log-tree.c|  37 ++---
 log-tree.h|   6 ++-
 pretty.c  |   4 +-
 revision.c|   2 +-
 t/t4202-log.sh| 101 ++
 7 files changed, 162 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 32246fdb0..314417d89 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -38,6 +38,18 @@ OPTIONS
are shown as if 'short' were given, otherwise no ref names are
shown. The default option is 'short'.
 
+--decorate-refs=::
+   Only print ref names that match the specified pattern. Uses the same
+   rules as `git rev-list --glob` except it doesn't assume a trailing a
+   trailing '/{asterisk}' if pattern lacks '?', '{asterisk}', or '['.
+   `--decorate-refs-exlclude` has precedence.
+
+--decorate-refs-exclude=::
+   Do not print ref names that match the specified pattern. Uses the same
+   rules as `git rev-list --glob` except it doesn't assume a trailing a
+   trailing '/{asterisk}' if pattern lacks '?', '{asterisk}', or '['.
+   Has precedence over `--decorate-refs`.
+
 --source::
Print out the ref name given on the command line by which each
commit was reached.
diff --git a/builtin/log.c b/builtin/log.c
index d81a09051..3587c0055 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -143,11 +143,19 @@ static void cmd_log_init_finish(int argc, const char 
**argv, const char *prefix,
struct userformat_want w;
int quiet = 0, source = 0, mailmap = 0;
static struct line_opt_callback_data line_cb = {NULL, NULL, 
STRING_LIST_INIT_DUP};
+   static struct string_list decorate_refs_exclude = STRING_LIST_INIT_DUP;
+   static struct string_list decorate_refs_include = STRING_LIST_INIT_DUP;
+   struct ref_include_exclude_list ref_filter_lists = 
{_refs_include,
+   
_refs_exclude};
 
const struct option builtin_log_options[] = {
OPT__QUIET(, N_("suppress diff output")),
OPT_BOOL(0, "source", , N_("show source")),
OPT_BOOL(0, "use-mailmap", , N_("Use mail map file")),
+   OPT_STRING_LIST(0, "decorate-refs", _refs_include,
+   N_("ref"), N_("only decorate these refs")),
+   OPT_STRING_LIST(0, "decorate-refs-exclude", 
_refs_exclude,
+   N_("ref"), N_("do not decorate these refs")),
{ OPTION_CALLBACK, 0, "decorate", NULL, NULL, N_("decorate 
options"),
  PARSE_OPT_OPTARG, decorate_callback},
OPT_CALLBACK('L', NULL, _cb, "n,m:file",
@@ -206,7 +214,7 @@ static void cmd_log_init_finish(int argc, const char 
**argv, const char *prefix,
 
if (decoration_style) {
rev->show_decorations = 1;
-   load_ref_decorations(decoration_style);
+   load_ref_decorations(decoration_style, _filter_lists);
}
 
if (rev->line_level_traverse)
diff --git a/log-tree.c b/log-tree.c
index cea056234..8efc7ac3d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -94,9 +94,33 @@ static int add_ref_decoration(const char *refname, const 
struct object_id *oid,
 {
struct object *obj;
enum decoration_type type = DECORATION_NONE;
+   struct ref_include_exclude_list *filter = (struct 
ref_include_exclude_list *)cb_data;
+   struct string_list_item *item;
+   struct strbuf real_pattern = STRBUF_INIT;
+
+   if(filter && filter->exclude->nr > 0) {
+   /* if current ref is on the exclude list skip */
+   for_each_string_list_item(item, filter->exclude) {
+   strbuf_reset(_pattern);
+   normalize_glob_ref(_pattern, NULL, item->string, 
0);
+