* Prereleases of Git for Windows
From: Johannes Schindelin @ 2016-11-19 15:10 UTC (permalink / raw)
To: git-for-windows; +Cc: git
Hi all,
I debated whether I should clutter the mailing list by announcing the
prereleases I published based on the v2.11.0-rc* releases of upstream Git.
I ended up deciding to announce that I won't announce them on the mailing
list, but only on Twitter [*1*]. After announcing the latest prerelease,
of course:
https://github.com/git-for-windows/git/releases/tag/v2.11.0-rc2.windows.1
Please give this a good beating and open tickets for issues you encounter
(unless you find that there are already open or closed tickets for the
same bug).
Thank you,
Johannes
Footnote: https://twitter.com/GitForWindows
^ permalink raw reply
* [PATCH v2 2/2] ref-filter: add support to display trailers as part of contents
From: Jacob Keller @ 2016-11-19 0:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
In-Reply-To: <20161119005815.3646-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>
Add %(trailers) and %(contents:trailers) to display the trailers as
interpreted by trailer_info_get. Update documentation and add a test for
the new feature.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/git-for-each-ref.txt | 2 ++
ref-filter.c | 22 +++++++++++++++++++++-
t/t6300-for-each-ref.sh | 26 ++++++++++++++++++++++++++
3 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index f57e69bc83e3..e5807eede787 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -165,6 +165,8 @@ of all lines of the commit message up to the first blank line. The next
line is 'contents:body', where body is all of the lines after the first
blank line. The optional GPG signature is `contents:signature`. The
first `N` lines of the message is obtained using `contents:lines=N`.
+Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]
+are obtained as 'contents:trailers'.
For sorting purposes, fields with numeric values sort in numeric order
(`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).
diff --git a/ref-filter.c b/ref-filter.c
index d4c2931f3aab..b6f1bb73ed37 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,7 @@
#include "utf8.h"
#include "git-compat-util.h"
#include "version.h"
+#include "trailer.h"
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
@@ -40,7 +41,7 @@ static struct used_atom {
enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
remote_ref;
struct {
- enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB } option;
+ enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB, C_TRAILERS } option;
unsigned int nlines;
} contents;
enum { O_FULL, O_SHORT } objectname;
@@ -85,6 +86,13 @@ static void subject_atom_parser(struct used_atom *atom, const char *arg)
atom->u.contents.option = C_SUB;
}
+static void trailers_atom_parser(struct used_atom *atom, const char *arg)
+{
+ if (arg)
+ die(_("%%(trailers) does not take arguments"));
+ atom->u.contents.option = C_TRAILERS;
+}
+
static void contents_atom_parser(struct used_atom *atom, const char *arg)
{
if (!arg)
@@ -95,6 +103,8 @@ static void contents_atom_parser(struct used_atom *atom, const char *arg)
atom->u.contents.option = C_SIG;
else if (!strcmp(arg, "subject"))
atom->u.contents.option = C_SUB;
+ else if (!strcmp(arg, "trailers"))
+ atom->u.contents.option = C_TRAILERS;
else if (skip_prefix(arg, "lines=", &arg)) {
atom->u.contents.option = C_LINES;
if (strtoul_ui(arg, 10, &atom->u.contents.nlines))
@@ -194,6 +204,7 @@ static struct {
{ "creatordate", FIELD_TIME },
{ "subject", FIELD_STR, subject_atom_parser },
{ "body", FIELD_STR, body_atom_parser },
+ { "trailers", FIELD_STR, trailers_atom_parser },
{ "contents", FIELD_STR, contents_atom_parser },
{ "upstream", FIELD_STR, remote_ref_atom_parser },
{ "push", FIELD_STR, remote_ref_atom_parser },
@@ -785,6 +796,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
name++;
if (strcmp(name, "subject") &&
strcmp(name, "body") &&
+ strcmp(name, "trailers") &&
!starts_with(name, "contents"))
continue;
if (!subpos)
@@ -808,6 +820,14 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
/* Size is the length of the message after removing the signature */
append_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);
v->s = strbuf_detach(&s, NULL);
+ } else if (atom->u.contents.option == C_TRAILERS) {
+ struct trailer_info info;
+
+ /* Search for trailer info */
+ trailer_info_get(&info, subpos);
+ v->s = xmemdupz(info.trailer_start,
+ info.trailer_end - info.trailer_start);
+ trailer_info_release(&info);
} else if (atom->u.contents.option == C_BARE)
v->s = xstrdup(subpos);
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 19a2823025e7..eb4bac0fe477 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -553,4 +553,30 @@ test_expect_success 'Verify sort with multiple keys' '
refs/tags/bogo refs/tags/master > actual &&
test_cmp expected actual
'
+
+cat >trailers <<EOF
+Reviewed-by: A U Thor <author@example.com>
+Signed-off-by: A U Thor <author@example.com>
+EOF
+
+test_expect_success 'basic atom: head contents:trailers' '
+ echo "Some contents" > two &&
+ git add two &&
+ git commit -F - <<-EOF &&
+ trailers: this commit message has trailers
+
+ Some message contents
+
+ $(cat trailers)
+ EOF
+ git for-each-ref --format="%(contents:trailers)" refs/heads/master >actual &&
+ sanitize_pgp <actual >actual.clean &&
+ # git for-each-ref ends with a blank line
+ cat >expect <<-EOF &&
+ $(cat trailers)
+
+ EOF
+ test_cmp expect actual.clean
+'
+
test_done
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* [PATCH v2 1/2] pretty: add %(trailers) format for displaying trailers of a commit message
From: Jacob Keller @ 2016-11-19 0:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
In-Reply-To: <20161119005815.3646-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>
Recent patches have expanded on the trailers.c code and we have the
builtin commant git-interpret-trailers which can be used to add or
modify trailer lines. However, there is no easy way to simply display
the trailers of a commit message.
Add support for %(trailers) format modifier which will use the
trailer_info_get() calls to read trailers in an identical way as git
interpret-trailers does. Use a long format option instead of a short
name so that future work can more easily unify ref-filter and pretty
formats.
Add documentation and tests for the same.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/pretty-formats.txt | 2 ++
pretty.c | 17 +++++++++++++++++
t/t4205-log-pretty-formats.sh | 26 ++++++++++++++++++++++++++
3 files changed, 45 insertions(+)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 3bcee2ddb124..47b286b33e4e 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -199,6 +199,8 @@ endif::git-rev-list[]
than given and there are spaces on its left, use those spaces
- '%><(<N>)', '%><|(<N>)': similar to '% <(<N>)', '%<|(<N>)'
respectively, but padding both sides (i.e. the text is centered)
+-%(trailers): display the trailers of the body as interpreted by
+ linkgit:git-interpret-trailers[1]
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index 37b2c3b1f995..5e683830d9d6 100644
--- a/pretty.c
+++ b/pretty.c
@@ -10,6 +10,7 @@
#include "color.h"
#include "reflog-walk.h"
#include "gpg-interface.h"
+#include "trailer.h"
static char *user_format;
static struct cmt_fmt_map {
@@ -889,6 +890,16 @@ const char *format_subject(struct strbuf *sb, const char *msg,
return msg;
}
+static void format_trailers(struct strbuf *sb, const char *msg)
+{
+ struct trailer_info info;
+
+ trailer_info_get(&info, msg);
+ strbuf_add(sb, info.trailer_start,
+ info.trailer_end - info.trailer_start);
+ trailer_info_release(&info);
+}
+
static void parse_commit_message(struct format_commit_context *c)
{
const char *msg = c->message + c->message_off;
@@ -1292,6 +1303,12 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
strbuf_addstr(sb, msg + c->body_off);
return 1;
}
+
+ if (starts_with(placeholder, "(trailers)")) {
+ format_trailers(sb, msg + c->subject_off);
+ return strlen("(trailers)");
+ }
+
return 0; /* unknown placeholder */
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index f5435fd250ba..21eb8c8587f2 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -535,4 +535,30 @@ test_expect_success 'clean log decoration' '
test_cmp expected actual1
'
+cat >trailers <<EOF
+Signed-off-by: A U Thor <author@example.com>
+Acked-by: A U Thor <author@example.com>
+[ v2 updated patch description ]
+Signed-off-by: A U Thor <author@example.com>
+EOF
+
+test_expect_success 'pretty format %(trailers) shows trailers' '
+ echo "Some contents" >trailerfile &&
+ git add trailerfile &&
+ git commit -F - <<-EOF &&
+ trailers: this commit message has trailers
+
+ This commit is a test commit with trailers at the end. We parse this
+ message and display the trailers using %bT
+
+ $(cat trailers)
+ EOF
+ git log --no-walk --pretty="%(trailers)" >actual &&
+ cat >expect <<-EOF &&
+ $(cat trailers)
+
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* [PATCH 0/2] add format specifiers to display trailers
From: Jacob Keller @ 2016-11-19 0:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
This is based off of jt/use-trailer-api-in-commands so that we can make
use of the public trailer API that will parse a string for trailers.
I use trailers as a way to store extra commit metadata, and would like a
convenient way to obtain the trailers of a commit message easily. This
adds format specifiers to both the ref-filter API and the pretty
format specifiers, using %(trailers) for both (and also
contents:trailers for ref-filter).
Additionally, I am somewhat not a fan of the way that if you have a
series of trailers which are trailer format, but not recognized, such
as the following:
<text>
My-tag: my value
My-other-tag: my other value
[non-trailer line]
My-tag: my third value
Git interpret-trailers will not recognize this as a trailer block
because it doesn't have any standard git tags within it.
Junio suggested that we should treat all the configured trailer prefixes
as recognized so that it would work as well, but it doesn't appear to
do this at least for jt/use-trailer-api-in-commands
I think that's the right solution, since it's extensible, though it
would mean that interpret-trailers would behave differently on different
systems... not really sure it's all bad though.
interdiff v1:
diff --git c/Documentation/pretty-formats.txt w/Documentation/pretty-formats.txt
index 9ee68a4cb64a..47b286b33e4e 100644
--- c/Documentation/pretty-formats.txt
+++ w/Documentation/pretty-formats.txt
@@ -138,7 +138,6 @@ The placeholders are:
- '%s': subject
- '%f': sanitized subject line, suitable for a filename
- '%b': body
-- '%bT': trailers of body as interpreted by linkgit:git-interpret-trailers[1]
- '%B': raw body (unwrapped subject and body)
ifndef::git-rev-list[]
- '%N': commit notes
@@ -200,6 +199,8 @@ endif::git-rev-list[]
than given and there are spaces on its left, use those spaces
- '%><(<N>)', '%><|(<N>)': similar to '% <(<N>)', '%<|(<N>)'
respectively, but padding both sides (i.e. the text is centered)
+-%(trailers): display the trailers of the body as interpreted by
+ linkgit:git-interpret-trailers[1]
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
diff --git c/pretty.c w/pretty.c
index ea8764334865..5e683830d9d6 100644
--- c/pretty.c
+++ w/pretty.c
@@ -1300,16 +1300,15 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
format_sanitized_subject(sb, msg + c->subject_off);
return 1;
case 'b': /* body */
- switch (placeholder[1]) {
- case 'T':
- format_trailers(sb, msg + c->subject_off);
- return 2;
- default:
- break;
- }
strbuf_addstr(sb, msg + c->body_off);
return 1;
}
+
+ if (starts_with(placeholder, "(trailers)")) {
+ format_trailers(sb, msg + c->subject_off);
+ return strlen("(trailers)");
+ }
+
return 0; /* unknown placeholder */
}
diff --git c/t/t4205-log-pretty-formats.sh w/t/t4205-log-pretty-formats.sh
index 7a35941ddcbd..21eb8c8587f2 100755
--- c/t/t4205-log-pretty-formats.sh
+++ w/t/t4205-log-pretty-formats.sh
@@ -542,7 +542,7 @@ Acked-by: A U Thor <author@example.com>
Signed-off-by: A U Thor <author@example.com>
EOF
-test_expect_success 'pretty format %bT shows trailers' '
+test_expect_success 'pretty format %(trailers) shows trailers' '
echo "Some contents" >trailerfile &&
git add trailerfile &&
git commit -F - <<-EOF &&
@@ -553,7 +553,7 @@ test_expect_success 'pretty format %bT shows trailers' '
$(cat trailers)
EOF
- git log --no-walk --pretty="%bT" >actual &&
+ git log --no-walk --pretty="%(trailers)" >actual &&
cat >expect <<-EOF &&
$(cat trailers)
Jacob Keller (2):
pretty: add %bT format for displaying trailers of a commit message
ref-filter: add support to display trailers as part of contents
Documentation/git-for-each-ref.txt | 2 ++
Documentation/pretty-formats.txt | 1 +
pretty.c | 18 ++++++++++++++++++
ref-filter.c | 22 +++++++++++++++++++++-
t/t4205-log-pretty-formats.sh | 26 ++++++++++++++++++++++++++
t/t6300-for-each-ref.sh | 26 ++++++++++++++++++++++++++
6 files changed, 94 insertions(+), 1 deletion(-)
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* Re: [PATCH 0/2] add format specifiers to display trailers
From: Jacob Keller @ 2016-11-18 23:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Jonathan Tan, Git mailing list
In-Reply-To: <xmqq8tsgl5o4.fsf@gitster.mtv.corp.google.com>
On Fri, Nov 18, 2016 at 3:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jacob Keller <jacob.e.keller@intel.com> writes:
>
>> Git interpret-trailers will not recognize this as a trailer block
>> because it doesn't have any standard git tags within it. Would it be ok
>> to augment the trailer interpretation to say that if we have over 75%
>> trailers in the block that we accept it even if it doesn't have any real
>> recognized tags?
>
> I thought the documented way to do this is to configure one of your
> custom trailer as such. Jonathan?
>
That would be fine then, if that works.
>> pretty: add %bT format for displaying trailers of a commit message
>
> Are %(...) taken already? In longer term, it would be nice if we
> can unify the --pretty formats and for-each-ref formats, so it is
> probably better if we avoid adding any new short ones to the former.
>
Oh, I hadn't considered adding a longer one. I'll rework this to use
longer ones.
> We have %s and %b so that we can reconstruct the whole thing by
> using both. It is unclear how %bT fits in this picture. I wonder
> if we also need another placeholder that expands to the body of the
> message without the trailer---otherwise the whole set would become
> incoherent, no?
>
I'm not entirely sure what to do here. I just wanted a way to easily
format "just the trailers" of a message. We could add something that
formats just the non-trailers, that's not too difficult. Not really
sure what I'd call it though.
Thanks,
Jake
^ permalink raw reply
* Fwd: git diff with “--word-diff-regex” extremely slow compared to “--word-diff”?
From: Matthieu S @ 2016-11-18 23:40 UTC (permalink / raw)
To: git
In-Reply-To: <CAEYvigJ14xYDmRG2N0yTgM4spaaB7s9923w0+e9+QQEeFz0NTQ@mail.gmail.com>
Hi
When giving a custom regex to git diff --word-diff-regex= instead of
using the default --word-diff (which splits words on whitespace), git
slows down very considerably... I don't understand why such a speed
difference?
(this question was asked on stack overflow, but after two month
without answer, I'm asking it here instead. Post:
http://stackoverflow.com/questions/39027864/git-diff-with-word-diff-regex-extremely-slow-compared-to-word-diff).
Example (sorry, UNIX specific code): create two one-line files, and
two 200000-lines files:
echo aaa,bbb ,12,12,15 >file1.txt
echo aaa,bbb ,12,12,16 >file2.txt
awk '{for(i=0;i<200000;i++)print}' file1.txt > file1BIG.txt
awk '{for(i=0;i<200000;i++)print}' file2.txt > file2BIG.txt
Default --word-diff has no issues with the BIG files (cannot see time
difference):
git diff --word-diff file1.txt file2.txt
git diff --word-diff file1BIG.txt file2BIG.txt
Now use instead --word-diff-regex= argument (with regex from post:
http://stackoverflow.com/questions/10482773/also-use-comma-as-a-word-separator-in-diff
)
git diff --word-diff-regex=[^[:space:],] file1.txt file2.txt
git diff --word-diff-regex=[^[:space:],] file1BIG.txt file2BIG.txt
Why is the speed so different if one uses --word-diff instead of
--word-diff-regex= ? Is it just because my expression is (slightly)
more complex than the default one (split on period instead of only
whitespace) ? Or is it that the default word-diff is implemented
differently/more efficiently? How can I overcome this speed slowdown?
Thanks!!
Matthieu
PS: using git 2.7.4 on Ubuntu 16.04
^ permalink raw reply
* Re: [PATCH 13/16] submodule: teach unpack_trees() to update submodules
From: Stefan Beller @ 2016-11-18 23:39 UTC (permalink / raw)
To: Brandon Williams
Cc: git@vger.kernel.org, Junio C Hamano, Jonathan Nieder, Martin Fick,
David Turner
In-Reply-To: <20161116002520.GI66382@google.com>
On Tue, Nov 15, 2016 at 4:25 PM, Brandon Williams <bmwill@google.com> wrote:
> On 11/15, Stefan Beller wrote:
>> + int flags = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE;
>
> For readability you may want to have spaces between the two flags
done
>
>> + if (o->index_only
>> + || (!((old->ce_flags & CE_VALID) || ce_skip_worktree(old))
>> + && (o->reset || ce_uptodate(old))))
>> + return 0;
>
> The coding guidelines say that git prefers to have the logical operators
> on the right side like this:
fixed all the coding style issues.
^ permalink raw reply
* Re: [PATCH 0/2] add format specifiers to display trailers
From: Junio C Hamano @ 2016-11-18 23:38 UTC (permalink / raw)
To: Jacob Keller, Jonathan Tan; +Cc: git, Jacob Keller
In-Reply-To: <20161118230825.20952-1-jacob.e.keller@intel.com>
Jacob Keller <jacob.e.keller@intel.com> writes:
> Git interpret-trailers will not recognize this as a trailer block
> because it doesn't have any standard git tags within it. Would it be ok
> to augment the trailer interpretation to say that if we have over 75%
> trailers in the block that we accept it even if it doesn't have any real
> recognized tags?
I thought the documented way to do this is to configure one of your
custom trailer as such. Jonathan?
> pretty: add %bT format for displaying trailers of a commit message
Are %(...) taken already? In longer term, it would be nice if we
can unify the --pretty formats and for-each-ref formats, so it is
probably better if we avoid adding any new short ones to the former.
We have %s and %b so that we can reconstruct the whole thing by
using both. It is unclear how %bT fits in this picture. I wonder
if we also need another placeholder that expands to the body of the
message without the trailer---otherwise the whole set would become
incoherent, no?
^ permalink raw reply
* Re: [PATCH 13/16] submodule: teach unpack_trees() to update submodules
From: Stefan Beller @ 2016-11-18 23:33 UTC (permalink / raw)
To: David Turner
Cc: git@vger.kernel.org, bmwill@google.com, gitster@pobox.com,
jrnieder@gmail.com, mogulguy10@gmail.com
In-Reply-To: <f54d446aa7734cb4aec4b51c7b81a2b6@exmbdft7.ad.twosigma.com>
On Tue, Nov 15, 2016 at 4:22 PM, David Turner <David.Turner@twosigma.com> wrote:
>> msgs[ERROR_NOT_UPTODATE_DIR] =
>> _("Updating the following directories would lose untracked
>> files in it:\n%s");
>> + msgs[ERROR_NOT_UPTODATE_SUBMODULE] =
>> + _("Updating the following submodules would lose modifications
>> in
>> +it:\n%s");
>
> s/it/them/
done, also fixed the existing ERROR_NOT_UPTODATE_DIR.
>> + if (!S_ISGITLINK(ce->ce_mode)) {
>
> I generally prefer to avoid if (!x) { A } else { B } -- I would rather just see if (x) { B } else { A }.
done.
>> + if (submodule_is_interesting(old->name, null_sha1)
>> + && ok_to_remove_submodule(old->name))
>> + return 0;
>> + }
>
> Do we need a return 1 in here somewhere? Because otherwise, we fall through and return 0 later.
Otherwise we would fall through and run
if (errno == ENOENT)
return 0;
return o->gently ? -1 :
add_rejected_path(o, error_type, ce->name);
which produces different results than 0?
^ permalink raw reply
* Re: [PATCH v7 00/17] port branch.c to use ref-filter's printing options
From: Junio C Hamano @ 2016-11-18 23:31 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Git List, Jacob Keller
In-Reply-To: <CAOLa=ZQtmQWpFMPa-SD29N7hASHAPp8SGGJsLu+AW_Kv-1LqwA@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
> Thanks, will add it in.
OK, here is a reroll of what I sent earlier in
http://public-inbox.org/git/<xmqq7f84tqa7.fsf_-_@gitster.mtv.corp.google.com>
but rebased so that it can happen as a preparatory bugfix before
your series.
The bug dates back to the very original implementation of %(HEAD) in
7a48b83219 ("for-each-ref: introduce %(HEAD) asterisk marker",
2013-11-18) and was moved to the current location in the v2.6 days
at c95b758587 ("ref-filter: move code from 'for-each-ref'",
2015-06-14).
-- >8 --
Subject: [PATCH] for-each-ref: do not segv with %(HEAD) on an unborn branch
The code to flip between "*" and " " prefixes depending on what
branch is checked out used in --format='%(HEAD)' did not consider
that HEAD may resolve to an unborn branch and dereferenced a NULL.
This will become a lot easier to trigger as the codepath will be
used to reimplement "git branch [--list]" in the future.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
ref-filter.c | 2 +-
t/t6300-for-each-ref.sh | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/ref-filter.c b/ref-filter.c
index bc551a752c..d7e91a78da 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1017,7 +1017,7 @@ static void populate_value(struct ref_array_item *ref)
head = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
sha1, NULL);
- if (!strcmp(ref->refname, head))
+ if (head && !strcmp(ref->refname, head))
v->s = "*";
else
v->s = " ";
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 19a2823025..039509a9cb 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -553,4 +553,14 @@ test_expect_success 'Verify sort with multiple keys' '
refs/tags/bogo refs/tags/master > actual &&
test_cmp expected actual
'
+
+test_expect_success 'do not dereference NULL upon %(HEAD) on unborn branch' '
+ test_when_finished "git checkout master" &&
+ git for-each-ref --format="%(HEAD) %(refname:short)" refs/heads/ >actual &&
+ sed -e "s/^\* / /" actual >expect &&
+ git checkout --orphan HEAD &&
+ git for-each-ref --format="%(HEAD) %(refname:short)" refs/heads/ >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0-rc2-152-gc9ad1dc38a
^ permalink raw reply related
* [PATCH 2/2] ref-filter: add support to display trailers as part of contents
From: Jacob Keller @ 2016-11-18 23:08 UTC (permalink / raw)
To: git; +Cc: Jacob Keller
In-Reply-To: <20161118230825.20952-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>
Add %(trailers) and %(contents:trailers) to display the trailers as
interpreted by trailer_info_get. Update documentation and add a test for
the new feature.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/git-for-each-ref.txt | 2 ++
ref-filter.c | 22 +++++++++++++++++++++-
t/t6300-for-each-ref.sh | 26 ++++++++++++++++++++++++++
3 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index f57e69bc83e3..e5807eede787 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -165,6 +165,8 @@ of all lines of the commit message up to the first blank line. The next
line is 'contents:body', where body is all of the lines after the first
blank line. The optional GPG signature is `contents:signature`. The
first `N` lines of the message is obtained using `contents:lines=N`.
+Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]
+are obtained as 'contents:trailers'.
For sorting purposes, fields with numeric values sort in numeric order
(`objectsize`, `authordate`, `committerdate`, `creatordate`, `taggerdate`).
diff --git a/ref-filter.c b/ref-filter.c
index d4c2931f3aab..b6f1bb73ed37 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -13,6 +13,7 @@
#include "utf8.h"
#include "git-compat-util.h"
#include "version.h"
+#include "trailer.h"
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
@@ -40,7 +41,7 @@ static struct used_atom {
enum { RR_NORMAL, RR_SHORTEN, RR_TRACK, RR_TRACKSHORT }
remote_ref;
struct {
- enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB } option;
+ enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB, C_TRAILERS } option;
unsigned int nlines;
} contents;
enum { O_FULL, O_SHORT } objectname;
@@ -85,6 +86,13 @@ static void subject_atom_parser(struct used_atom *atom, const char *arg)
atom->u.contents.option = C_SUB;
}
+static void trailers_atom_parser(struct used_atom *atom, const char *arg)
+{
+ if (arg)
+ die(_("%%(trailers) does not take arguments"));
+ atom->u.contents.option = C_TRAILERS;
+}
+
static void contents_atom_parser(struct used_atom *atom, const char *arg)
{
if (!arg)
@@ -95,6 +103,8 @@ static void contents_atom_parser(struct used_atom *atom, const char *arg)
atom->u.contents.option = C_SIG;
else if (!strcmp(arg, "subject"))
atom->u.contents.option = C_SUB;
+ else if (!strcmp(arg, "trailers"))
+ atom->u.contents.option = C_TRAILERS;
else if (skip_prefix(arg, "lines=", &arg)) {
atom->u.contents.option = C_LINES;
if (strtoul_ui(arg, 10, &atom->u.contents.nlines))
@@ -194,6 +204,7 @@ static struct {
{ "creatordate", FIELD_TIME },
{ "subject", FIELD_STR, subject_atom_parser },
{ "body", FIELD_STR, body_atom_parser },
+ { "trailers", FIELD_STR, trailers_atom_parser },
{ "contents", FIELD_STR, contents_atom_parser },
{ "upstream", FIELD_STR, remote_ref_atom_parser },
{ "push", FIELD_STR, remote_ref_atom_parser },
@@ -785,6 +796,7 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
name++;
if (strcmp(name, "subject") &&
strcmp(name, "body") &&
+ strcmp(name, "trailers") &&
!starts_with(name, "contents"))
continue;
if (!subpos)
@@ -808,6 +820,14 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct obj
/* Size is the length of the message after removing the signature */
append_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);
v->s = strbuf_detach(&s, NULL);
+ } else if (atom->u.contents.option == C_TRAILERS) {
+ struct trailer_info info;
+
+ /* Search for trailer info */
+ trailer_info_get(&info, subpos);
+ v->s = xmemdupz(info.trailer_start,
+ info.trailer_end - info.trailer_start);
+ trailer_info_release(&info);
} else if (atom->u.contents.option == C_BARE)
v->s = xstrdup(subpos);
}
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 19a2823025e7..eb4bac0fe477 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -553,4 +553,30 @@ test_expect_success 'Verify sort with multiple keys' '
refs/tags/bogo refs/tags/master > actual &&
test_cmp expected actual
'
+
+cat >trailers <<EOF
+Reviewed-by: A U Thor <author@example.com>
+Signed-off-by: A U Thor <author@example.com>
+EOF
+
+test_expect_success 'basic atom: head contents:trailers' '
+ echo "Some contents" > two &&
+ git add two &&
+ git commit -F - <<-EOF &&
+ trailers: this commit message has trailers
+
+ Some message contents
+
+ $(cat trailers)
+ EOF
+ git for-each-ref --format="%(contents:trailers)" refs/heads/master >actual &&
+ sanitize_pgp <actual >actual.clean &&
+ # git for-each-ref ends with a blank line
+ cat >expect <<-EOF &&
+ $(cat trailers)
+
+ EOF
+ test_cmp expect actual.clean
+'
+
test_done
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* [PATCH 0/2] add format specifiers to display trailers
From: Jacob Keller @ 2016-11-18 23:08 UTC (permalink / raw)
To: git; +Cc: Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
This is based off of jt/use-trailer-api-in-commands so that we can make
use of the public trailer API that will parse a string for trailers.
I use trailers as a way to store extra commit metadata, and would like a
convenient way to obtain the trailers of a commit message easily. This
adds format specifiers to both the ref-filter API and the pretty
formats. I am not a fan of %bT but %t and %T were already taken. I don't
really know if it's ok to use %bT, since I think we used to allow "%bT"
format, though i don't think this is likely used much in practice.
I am open to suggestions for the pretty format specifier.
Additionally, I am somewhat not a fan of the way that if you have a
series of trailers which are trailer format, but not recognized, such
as the following:
<text>
My-tag: my value
My-other-tag: my other value
[non-trailer line]
My-tag: my third value
---
Git interpret-trailers will not recognize this as a trailer block
because it doesn't have any standard git tags within it. Would it be ok
to augment the trailer interpretation to say that if we have over 75%
trailers in the block that we accept it even if it doesn't have any real
recognized tags?
I say this because I regularly use extra tags in my git projects to
represent change metadata, and it would be nice if the tag block could
be recognized even if it has 1-2 lines of non-trailer formatting in
it...
Thoughts?
Jacob Keller (2):
pretty: add %bT format for displaying trailers of a commit message
ref-filter: add support to display trailers as part of contents
Documentation/git-for-each-ref.txt | 2 ++
Documentation/pretty-formats.txt | 1 +
pretty.c | 18 ++++++++++++++++++
ref-filter.c | 22 +++++++++++++++++++++-
t/t4205-log-pretty-formats.sh | 26 ++++++++++++++++++++++++++
t/t6300-for-each-ref.sh | 26 ++++++++++++++++++++++++++
6 files changed, 94 insertions(+), 1 deletion(-)
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply
* [PATCH 1/2] pretty: add %bT format for displaying trailers of a commit message
From: Jacob Keller @ 2016-11-18 23:08 UTC (permalink / raw)
To: git; +Cc: Jacob Keller
In-Reply-To: <20161118230825.20952-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>
Recent patches have expanded on the trailers.c code and we have the
builtin commant git-interpret-trailers which can be used to add or
modify trailer lines. However, there is no easy way to simply display
the trailers of a commit message. Add support for %bT format modifier
which will use the trailer_info_get() calls to read trailers in an
identical way as git interpret-trailers does.
Add documentation and tests for the same.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/pretty-formats.txt | 1 +
pretty.c | 18 ++++++++++++++++++
t/t4205-log-pretty-formats.sh | 26 ++++++++++++++++++++++++++
3 files changed, 45 insertions(+)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 3bcee2ddb124..9ee68a4cb64a 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -138,6 +138,7 @@ The placeholders are:
- '%s': subject
- '%f': sanitized subject line, suitable for a filename
- '%b': body
+- '%bT': trailers of body as interpreted by linkgit:git-interpret-trailers[1]
- '%B': raw body (unwrapped subject and body)
ifndef::git-rev-list[]
- '%N': commit notes
diff --git a/pretty.c b/pretty.c
index 37b2c3b1f995..ea8764334865 100644
--- a/pretty.c
+++ b/pretty.c
@@ -10,6 +10,7 @@
#include "color.h"
#include "reflog-walk.h"
#include "gpg-interface.h"
+#include "trailer.h"
static char *user_format;
static struct cmt_fmt_map {
@@ -889,6 +890,16 @@ const char *format_subject(struct strbuf *sb, const char *msg,
return msg;
}
+static void format_trailers(struct strbuf *sb, const char *msg)
+{
+ struct trailer_info info;
+
+ trailer_info_get(&info, msg);
+ strbuf_add(sb, info.trailer_start,
+ info.trailer_end - info.trailer_start);
+ trailer_info_release(&info);
+}
+
static void parse_commit_message(struct format_commit_context *c)
{
const char *msg = c->message + c->message_off;
@@ -1289,6 +1300,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
format_sanitized_subject(sb, msg + c->subject_off);
return 1;
case 'b': /* body */
+ switch (placeholder[1]) {
+ case 'T':
+ format_trailers(sb, msg + c->subject_off);
+ return 2;
+ default:
+ break;
+ }
strbuf_addstr(sb, msg + c->body_off);
return 1;
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index f5435fd250ba..7a35941ddcbd 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -535,4 +535,30 @@ test_expect_success 'clean log decoration' '
test_cmp expected actual1
'
+cat >trailers <<EOF
+Signed-off-by: A U Thor <author@example.com>
+Acked-by: A U Thor <author@example.com>
+[ v2 updated patch description ]
+Signed-off-by: A U Thor <author@example.com>
+EOF
+
+test_expect_success 'pretty format %bT shows trailers' '
+ echo "Some contents" >trailerfile &&
+ git add trailerfile &&
+ git commit -F - <<-EOF &&
+ trailers: this commit message has trailers
+
+ This commit is a test commit with trailers at the end. We parse this
+ message and display the trailers using %bT
+
+ $(cat trailers)
+ EOF
+ git log --no-walk --pretty="%bT" >actual &&
+ cat >expect <<-EOF &&
+ $(cat trailers)
+
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* Re: [PATCH v4 4/6] grep: optionally recurse into submodules
From: Brandon Williams @ 2016-11-18 22:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sbeller, jonathantanmy
In-Reply-To: <xmqqlgwgl9ke.fsf@gitster.mtv.corp.google.com>
On 11/18, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
> > +static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec,
> > + int cached)
> > {
> > int hit = 0;
> > int nr;
> > + struct strbuf name = STRBUF_INIT;
> > + int name_base_len = 0;
> > + if (super_prefix) {
> > + name_base_len = strlen(super_prefix);
> > + strbuf_addstr(&name, super_prefix);
> > + }
> > +
> > read_cache();
> >
> > for (nr = 0; nr < active_nr; nr++) {
> > const struct cache_entry *ce = active_cache[nr];
> > - if (!S_ISREG(ce->ce_mode))
> > - continue;
> > - if (!ce_path_match(ce, pathspec, NULL))
> > - continue;
> > - /*
> > - * If CE_VALID is on, we assume worktree file and its cache entry
> > - * are identical, even if worktree file has been modified, so use
> > - * cache version instead
> > - */
> > - if (cached || (ce->ce_flags & CE_VALID) || ce_skip_worktree(ce)) {
> > - if (ce_stage(ce) || ce_intent_to_add(ce))
> > - continue;
> > - hit |= grep_sha1(opt, ce->oid.hash, ce->name, 0,
> > - ce->name);
> > + strbuf_setlen(&name, name_base_len);
> > + strbuf_addstr(&name, ce->name);
> > +
> > + if (S_ISREG(ce->ce_mode) &&
> > + match_pathspec(pathspec, name.buf, name.len, 0, NULL,
> > + S_ISDIR(ce->ce_mode) ||
> > + S_ISGITLINK(ce->ce_mode))) {
> > + /*
> > + * If CE_VALID is on, we assume worktree file and its
> > + * cache entry are identical, even if worktree file has
> > + * been modified, so use cache version instead
> > + */
> > + if (cached || (ce->ce_flags & CE_VALID) ||
> > + ce_skip_worktree(ce)) {
> > + if (ce_stage(ce) || ce_intent_to_add(ce))
> > + continue;
> > + hit |= grep_sha1(opt, ce->oid.hash, ce->name,
> > + 0, ce->name);
> > + } else {
> > + hit |= grep_file(opt, ce->name);
> > + }
> > + } else if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
> > + submodule_path_match(pathspec, name.buf, NULL)) {
> > + hit |= grep_submodule(opt, NULL, ce->name, ce->name);
> > }
> > - else
> > - hit |= grep_file(opt, ce->name);
>
> We used to reject anything other than S_ISREG() upfront in the loop,
> and then either did grep_sha1() from the cache or from grep_file()
> from the working tree.
>
> Now, the guard upfront is removed, and we do the same in the first
> part of this if/elseif. The elseif part deals with a submodule that
> could match the pathspec.
>
> Don't we need a final else clause that would skip the remainder of
> this loop? What would happen to a S_ISREG() path that does *NOT*
> match the given pathspec? We used to just "continue", but it seems
> to me that such a path will fall through the above if/elseif in the
> new code. Would that be a problem?
It may be (Though I didn't see any issues when running tests). It would
be easy enough to add an 'else continue;' at the end though.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v4 3/6] grep: add submodules as a grep source type
From: Brandon Williams @ 2016-11-18 22:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sbeller, jonathantanmy
In-Reply-To: <xmqq60nkmpud.fsf@gitster.mtv.corp.google.com>
On 11/18, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
> > diff --git a/grep.h b/grep.h
> > index 5856a23..267534c 100644
> > --- a/grep.h
> > +++ b/grep.h
> > @@ -161,6 +161,7 @@ struct grep_source {
> > GREP_SOURCE_SHA1,
> > GREP_SOURCE_FILE,
> > GREP_SOURCE_BUF,
> > + GREP_SOURCE_SUBMODULE,
> > } type;
> > void *identifier;
>
> Hmph, interesting. We have avoided ending enum definition with a
> comma, because it is only valid in more recent C than what we aim to
> support. This patch is not introducing a new problem, but just
> doing the same thing that would have broken older compilers as the
> existing code. Perhaps those older compilers have died out?
Perhaps it is time to move to a new C standard! :P
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v4 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Brandon Williams @ 2016-11-18 22:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sbeller, jonathantanmy
In-Reply-To: <xmqqh974l9bz.fsf@gitster.mtv.corp.google.com>
On 11/18, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
> > @@ -671,12 +707,29 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
> > enum interesting match = entry_not_interesting;
> > struct name_entry entry;
> > int old_baselen = base->len;
> > + struct strbuf name = STRBUF_INIT;
> > + int name_base_len = 0;
> > + if (super_prefix) {
> > + strbuf_addstr(&name, super_prefix);
> > + name_base_len = name.len;
> > + }
> >
> > while (tree_entry(tree, &entry)) {
> > int te_len = tree_entry_len(&entry);
> >
> > if (match != all_entries_interesting) {
> > - match = tree_entry_interesting(&entry, base, tn_len, pathspec);
> > + strbuf_setlen(&name, name_base_len);
> > + strbuf_addstr(&name, base->buf + tn_len);
> > +
> > + if (recurse_submodules && S_ISGITLINK(entry.mode)) {
> > + strbuf_addstr(&name, entry.path);
> > + match = submodule_path_match(pathspec, name.buf,
> > + NULL);
>
> The vocabulary from submodule_path_match() returns is the same as
> that of do_match_pathspec() and match_pathspec_item() which is
> MATCHED_{EXACTLY,FNMATCH,RECURSIVELY}, which is different from the
> vocabulary of the variable "match" which is "enum interesting" that
> is used by the tree-walk infrastructure.
>
> I doubt they are compatible to be usable like this. Am I missing
> something?
I think i initially must have thought it would work out, but looking
back at this I can clearly see that they aren't 100% compatible...
It slightly feels odd to me that we have so many different means for
checking pathspecs, all of which pretty much duplicate some of the
functionality of the other. Is there any reason there are these two
different code paths? Do we want them to remain separate or have them
be unified at some point?
Also, in order to use the tree_entry_interesting code it looks like I'll
either have to pipe through a flag saying 'yes i want to match against
submodules' like I did for the other pathspec codepath. Either that or
add functionality to perform wildmatching against partial matches (ie
directories and submodules) since currently the tree_entry_interesting
code path just punts and says 'well say it matches for now and check
again later' whenever it runs into a directory (I can't really make it
do that for submodules without a flag of somesort as tests could break).
Or maybe both?
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v7 14/17] ref-filter: allow porcelain to translate messages in the output
From: Jakub Narębski @ 2016-11-18 22:46 UTC (permalink / raw)
To: Karthik Nayak, git; +Cc: Jacob Keller
In-Reply-To: <20161108201211.25213-15-Karthik.188@gmail.com>
W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
> From: Karthik Nayak <karthik.188@gmail.com>
>
> Introduce setup_ref_filter_porcelain_msg() so that the messages used in
> the atom %(upstream:track) can be translated if needed. This is needed
> as we port branch.c to use ref-filter's printing API's.
>
> Written-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> ref-filter.c | 28 ++++++++++++++++++++++++----
> ref-filter.h | 2 ++
> 2 files changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index b47b900..944671a 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -15,6 +15,26 @@
> #include "version.h"
> #include "wt-status.h"
>
> +static struct ref_msg {
> + const char *gone;
> + const char *ahead;
> + const char *behind;
> + const char *ahead_behind;
> +} msgs = {
> + "gone",
> + "ahead %d",
> + "behind %d",
> + "ahead %d, behind %d"
> +};
> +
> +void setup_ref_filter_porcelain_msg(void)
> +{
> + msgs.gone = _("gone");
> + msgs.ahead = _("ahead %d");
> + msgs.behind = _("behind %d");
> + msgs.ahead_behind = _("ahead %d, behind %d");
> +}
Do I understand it correctly that this mechanism is here to avoid
repeated calls into gettext, as those messages would get repeated
over and over; otherwise one would use foo = N_("...") and _(foo),
isn't it?
I wonder if there is some way to avoid duplication here, but I don't
see anything easy and safe (e.g. against running setup_*() twice).
Best,
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v4 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Junio C Hamano @ 2016-11-18 22:19 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, jonathantanmy
In-Reply-To: <1479499135-64269-6-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> @@ -671,12 +707,29 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
> enum interesting match = entry_not_interesting;
> struct name_entry entry;
> int old_baselen = base->len;
> + struct strbuf name = STRBUF_INIT;
> + int name_base_len = 0;
> + if (super_prefix) {
> + strbuf_addstr(&name, super_prefix);
> + name_base_len = name.len;
> + }
>
> while (tree_entry(tree, &entry)) {
> int te_len = tree_entry_len(&entry);
>
> if (match != all_entries_interesting) {
> - match = tree_entry_interesting(&entry, base, tn_len, pathspec);
> + strbuf_setlen(&name, name_base_len);
> + strbuf_addstr(&name, base->buf + tn_len);
> +
> + if (recurse_submodules && S_ISGITLINK(entry.mode)) {
> + strbuf_addstr(&name, entry.path);
> + match = submodule_path_match(pathspec, name.buf,
> + NULL);
The vocabulary from submodule_path_match() returns is the same as
that of do_match_pathspec() and match_pathspec_item() which is
MATCHED_{EXACTLY,FNMATCH,RECURSIVELY}, which is different from the
vocabulary of the variable "match" which is "enum interesting" that
is used by the tree-walk infrastructure.
I doubt they are compatible to be usable like this. Am I missing
something?
> + } else {
> + match = tree_entry_interesting(&entry, &name,
> + 0, pathspec);
> + }
> +
> if (match == all_entries_not_interesting)
> break;
> if (match == entry_not_interesting)
^ permalink raw reply
* Re: [PATCH v4 4/6] grep: optionally recurse into submodules
From: Junio C Hamano @ 2016-11-18 22:14 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, jonathantanmy
In-Reply-To: <1479499135-64269-5-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> +static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec,
> + int cached)
> {
> int hit = 0;
> int nr;
> + struct strbuf name = STRBUF_INIT;
> + int name_base_len = 0;
> + if (super_prefix) {
> + name_base_len = strlen(super_prefix);
> + strbuf_addstr(&name, super_prefix);
> + }
> +
> read_cache();
>
> for (nr = 0; nr < active_nr; nr++) {
> const struct cache_entry *ce = active_cache[nr];
> - if (!S_ISREG(ce->ce_mode))
> - continue;
> - if (!ce_path_match(ce, pathspec, NULL))
> - continue;
> - /*
> - * If CE_VALID is on, we assume worktree file and its cache entry
> - * are identical, even if worktree file has been modified, so use
> - * cache version instead
> - */
> - if (cached || (ce->ce_flags & CE_VALID) || ce_skip_worktree(ce)) {
> - if (ce_stage(ce) || ce_intent_to_add(ce))
> - continue;
> - hit |= grep_sha1(opt, ce->oid.hash, ce->name, 0,
> - ce->name);
> + strbuf_setlen(&name, name_base_len);
> + strbuf_addstr(&name, ce->name);
> +
> + if (S_ISREG(ce->ce_mode) &&
> + match_pathspec(pathspec, name.buf, name.len, 0, NULL,
> + S_ISDIR(ce->ce_mode) ||
> + S_ISGITLINK(ce->ce_mode))) {
> + /*
> + * If CE_VALID is on, we assume worktree file and its
> + * cache entry are identical, even if worktree file has
> + * been modified, so use cache version instead
> + */
> + if (cached || (ce->ce_flags & CE_VALID) ||
> + ce_skip_worktree(ce)) {
> + if (ce_stage(ce) || ce_intent_to_add(ce))
> + continue;
> + hit |= grep_sha1(opt, ce->oid.hash, ce->name,
> + 0, ce->name);
> + } else {
> + hit |= grep_file(opt, ce->name);
> + }
> + } else if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
> + submodule_path_match(pathspec, name.buf, NULL)) {
> + hit |= grep_submodule(opt, NULL, ce->name, ce->name);
> }
> - else
> - hit |= grep_file(opt, ce->name);
We used to reject anything other than S_ISREG() upfront in the loop,
and then either did grep_sha1() from the cache or from grep_file()
from the working tree.
Now, the guard upfront is removed, and we do the same in the first
part of this if/elseif. The elseif part deals with a submodule that
could match the pathspec.
Don't we need a final else clause that would skip the remainder of
this loop? What would happen to a S_ISREG() path that does *NOT*
match the given pathspec? We used to just "continue", but it seems
to me that such a path will fall through the above if/elseif in the
new code. Would that be a problem?
^ permalink raw reply
* Re: [PATCH v4 4/6] grep: optionally recurse into submodules
From: Junio C Hamano @ 2016-11-18 22:01 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, jonathantanmy
In-Reply-To: <1479499135-64269-5-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> @@ -300,6 +311,10 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
> if (opt->relative && opt->prefix_length) {
> quote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);
> strbuf_insert(&pathbuf, 0, filename, tree_name_len);
> + } else if (super_prefix) {
> + strbuf_add(&pathbuf, filename, tree_name_len);
> + strbuf_addstr(&pathbuf, super_prefix);
> + strbuf_addstr(&pathbuf, filename + tree_name_len);
> } else {
> strbuf_addstr(&pathbuf, filename);
> }
> @@ -328,10 +343,13 @@ static int grep_file(struct grep_opt *opt, const char *filename)
> {
> struct strbuf buf = STRBUF_INIT;
>
> - if (opt->relative && opt->prefix_length)
> + if (opt->relative && opt->prefix_length) {
> quote_path_relative(filename, opt->prefix, &buf);
> - else
> + } else {
> + if (super_prefix)
> + strbuf_addstr(&buf, super_prefix);
> strbuf_addstr(&buf, filename);
> + }
The above two hunks both assume that the super_prefix option is
usable only from the top-level (i.e. opt->prefix_length == 0) and
also "--no-full-name" (which is the default) cannot be used. The
only invoker that runs "grep" with "--super-prefix" is the "grep"
that runs in the superproject, and it will only run us from the
top-level of the working tree, so the former assumption is OK.
It is a bit unclear to me how the "relative" and "--recurse-submodules"
would interact with each other, though.
^ permalink raw reply
* Re: [PATCH v7 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms
From: Jakub Narębski @ 2016-11-18 21:49 UTC (permalink / raw)
To: Junio C Hamano, Jacob Keller; +Cc: Karthik Nayak, Git mailing list
In-Reply-To: <xmqq4m38vdw4.fsf@gitster.mtv.corp.google.com>
W dniu 15.11.2016 o 18:42, Junio C Hamano pisze:
> Jacob Keller <jacob.keller@gmail.com> writes:
>
>> dirname makes sense. What about implementing a reverse variant of
>> strip, which you could perform stripping of right-most components and
>> instead of stripping by a number, strip "to" a number, ie: keep the
>> left N most components, and then you could use something like
>> ...
>> I think that would be more general purpose than basename, and less confusing?
>
> I think you are going in the right direction. I had a similar
> thought but built around a different axis. I.e. if strip=1 strips
> one from the left, perhaps we want to have rstrip=1 that strips one
> from the right, and also strip=-1 to mean strip everything except
> one from the left and so on?. I think this and your keep (and
> perhaps you'll have rkeep for completeness) have the same expressive
> power. I do not offhand have a preference one over the other.
>
> Somehow it sounds a bit strange to me to treat 'remotes' as the same
> class of token as 'heads' and 'tags' (I'd expect 'heads' and
> 'remotes/origin' would be at the same level in end-user's mind), but
> that is probably an unrelated tangent. The reason this series wants
> to introduce :base must be to emulate an existing feature, so that
> existing feature is a concrete counter-example that argues against
> my "it sounds a bit strange" reaction.
If it is to implement the feature where we select if to display only
local branches (refs/heads/**), only remote-tracking branches
(refs/remotes/**/**), or only tags (refs/tags/**), then perhaps
':category' or ':type' would make sense?
As in '%(refname:category)', e.g.
%(if:equals=heads)%(refname:category)%(then)...%(end)
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v4 4/6] grep: optionally recurse into submodules
From: Junio C Hamano @ 2016-11-18 21:48 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, jonathantanmy
In-Reply-To: <1479499135-64269-5-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> +static void compile_submodule_options(const struct grep_opt *opt,
> + const struct pathspec *pathspec,
> + int cached, int untracked,
> + int opt_exclude, int use_index,
> + int pattern_type_arg)
> +{
> + struct grep_pat *pattern;
> + int i;
> +
> + if (recurse_submodules)
> + argv_array_push(&submodule_options, "--recurse-submodules");
> +
> + if (cached)
> + argv_array_push(&submodule_options, "--cached");
> +...
> +
> + /* Add Pathspecs */
> + argv_array_push(&submodule_options, "--");
> + for (i = 0; i < pathspec->nr; i++)
> + argv_array_push(&submodule_options,
> + pathspec->items[i].original);
> +}
When I do
$ git grep --recurse-submodules pattern submodules/ lib/
where I have bunch of submodules in "submodules/" directory in the
top-level project, the top-level grep would try to find the pattern
in its own files in its "lib/" directory and then invoke sub-greps
in the submodule/a, submodule/b, etc. working trees.
This passes the "submodules/" and "lib/" pathspec down to these
sub-greps. These sub-greps in turn learn via --super-prefix where
they are in the super-project's context (e.g. "submodules/a/") to
adjust the given pathspec patterns, so everything cancels out
(e.g. they know "lib/" is totally outside of their area and their
files do not match with the pathspec element "lib/" at all).
Looking good.
^ permalink raw reply
* Re: [PATCH v4 3/6] grep: add submodules as a grep source type
From: Junio C Hamano @ 2016-11-18 21:37 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, jonathantanmy
In-Reply-To: <1479499135-64269-4-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> diff --git a/grep.h b/grep.h
> index 5856a23..267534c 100644
> --- a/grep.h
> +++ b/grep.h
> @@ -161,6 +161,7 @@ struct grep_source {
> GREP_SOURCE_SHA1,
> GREP_SOURCE_FILE,
> GREP_SOURCE_BUF,
> + GREP_SOURCE_SUBMODULE,
> } type;
> void *identifier;
Hmph, interesting. We have avoided ending enum definition with a
comma, because it is only valid in more recent C than what we aim to
support. This patch is not introducing a new problem, but just
doing the same thing that would have broken older compilers as the
existing code. Perhaps those older compilers have died out?
^ permalink raw reply
* Re: [PATCH v7 10/17] ref-filter: introduce refname_atom_parser_internal()
From: Jakub Narębski @ 2016-11-18 21:36 UTC (permalink / raw)
To: Karthik Nayak, git; +Cc: Jacob Keller
In-Reply-To: <20161108201211.25213-11-Karthik.188@gmail.com>
W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
> From: Karthik Nayak <karthik.188@gmail.com>
>
> Since there are multiple atoms which print refs ('%(refname)',
> '%(symref)', '%(push)', '%upstream'), it makes sense to have a common
Minor typo; it should be: "%(upstream)"
> ground for parsing them. This would allow us to share implementations of
> the atom modifiers between these atoms.
>
> Introduce refname_atom_parser_internal() to act as a common parsing
> function for ref printing atoms. This would eventually be used to
> introduce refname_atom_parser() and symref_atom_parser() and also be
> internally used in remote_ref_atom_parser().
>
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
> ---
[...]
> +static void refname_atom_parser_internal(struct refname_atom *atom,
> + const char *arg, const char *name)
> +{
> + if (!arg)
> + atom->option = R_NORMAL;
> + else if (!strcmp(arg, "short"))
> + atom->option = R_SHORT;
> + else if (skip_prefix(arg, "strip=", &arg)) {
> + atom->option = R_STRIP;
> + if (strtoul_ui(arg, 10, &atom->strip) || atom->strip <= 0)
> + die(_("positive value expected refname:strip=%s"), arg);
> + } else
^^^^^^
It looks like you have spurious tab here.
> + die(_("unrecognized %%(%s) argument: %s"), name, arg);
> +}
> +
> static void remote_ref_atom_parser(struct used_atom *atom, const char *arg)
> {
> struct string_list params = STRING_LIST_INIT_DUP;
>
^ permalink raw reply
* Re: [PATCH v7 09/17] ref-filter: make "%(symref)" atom work with the ':short' modifier
From: Jakub Narębski @ 2016-11-18 21:34 UTC (permalink / raw)
To: Karthik Nayak, git; +Cc: Jacob Keller
In-Reply-To: <20161108201211.25213-10-Karthik.188@gmail.com>
W dniu 08.11.2016 o 21:12, Karthik Nayak pisze:
[...]
> Add tests for %(symref) and %(symref:short) while we're here.
That's nice.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
> ---
[...]
> +test_expect_success 'Add symbolic ref for the following tests' '
> + git symbolic-ref refs/heads/sym refs/heads/master
> +'
> +
> +cat >expected <<EOF
> +refs/heads/master
> +EOF
This should be inside the relevant test, not outside. In other
patches in this series you are putting setup together with the
rest of test, by using "cat >expected <<-\EOF".
> +
> +test_expect_success 'Verify usage of %(symref) atom' '
> + git for-each-ref --format="%(symref)" refs/heads/sym > actual &&
This should be spelled " >actual", rather than " > actual"; there
should be no space between redirection and file name.
> + test_cmp expected actual
> +'
> +
> +cat >expected <<EOF
> +heads/master
> +EOF
> +
> +test_expect_success 'Verify usage of %(symref:short) atom' '
> + git for-each-ref --format="%(symref:short)" refs/heads/sym > actual &&
> + test_cmp expected actual
> +'
Same here.
> +
> test_done
>
--
Jakub Narębski
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox