Nguyễn Thái Ngọc Duy <pclo...@gmail.com> writes:
> Similar to --author/--committer which filters commits by author and
> committer header fields. --grep-reflog adds a fake "reflog" header to
> commit and a grep filter to search on that line.
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
> On Fri, Sep 28, 2012 at 12:09 AM, Junio C Hamano <gits...@pobox.com>
> > I am debating myself if it is sane for this option to have no hint
> > that it is about "limiting" in its name. "--author/--committer"
> > don't and it is clear from the context of the command that they are
> > not about setting author/committer, so "--reflog-message" may be
> > interpreted the same, perhaps.
> > The entry in the context above talks about multiple occurrence of
> > that option. Shouldn't this new one also say what happens when it
> > is
> > given twice?
> > I think I wrote prep_header_patterns() and compile_grep_patterns()
> > carefully enough not to assume the headers are only the author and
> > committer names, so the various combinations i.e. all-match,
> > author(s), committer(s), grep(s), and reflog-message(s), should
> > work
> > out of the box, but have you actually tested them?
> I did not. I do now, tests are also added.
You seem to have skipped a lot more important comment from the
message that came immediately after the above part. Did you take a
look at grep.c::match_one_pattern() that calls strip_timestamp() for
any and all GREP_PATTERN_HEAD items?
This is why I think that the [1/3] in this series shows a total lack
of design taste. Each element in "headers", unlike "body", is a
structured field and its payload has meaning depending on what field
name is associated with it. That is why we do not give a way to
users to willy-nilly add ad-hoc header fields.
The other side of this coin is that Git ought to be able to take
advantage of the meaning of each header. Stripping the timestamp
part when doing a textual match against --author/--committer is one
example. The code in match_one_pattern() to deal with the header
patterns need to be _extended_ to take into account the special
treatment each header, includign this new "reflog " fake header,
wants when doing a textual match, and when we add any other match
against header (either real or fake), we should be able to add
specific matching logic tailored to handle the particular meaning of
the header would have.
For that to happen, the code _must_ know what kind of headers we
would support; discarding the existing enum is going in a wrong
When we introduce "git log --header=frotz:xyzzy" option that looks
for a generic "frotz " header and tries to see if "xyzzy" textually
appears in the field, we may want to add a generic "this is not a
header that we have special treatment for" enum to the mix. But for
known kinds of headers, we would need a list of what each header is
and what semantics it wants.
So please reconsider undoing [1/3], and rerolling [2/3] that depends
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