* Re: [PATCH] terminology tweak: prune -> path limiting
From: Junio C Hamano @ 2018-12-09 1:52 UTC (permalink / raw)
To: Matthew DeVore; +Cc: git, jrn, matvore, dstolee, pclouds, peff
In-Reply-To: <xmqqo99v5vnc.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> AFAIK, "prune" is also used to describe unreachable loose objects,
s/describe/& the act of culling/
> but that use is fairly isolated and have little risk of being
> confusing too much. Are there other uses to make you consider it
> "highly overloaded"?
^ permalink raw reply
* Re: [PATCH on master v2] revision: use commit graph in get_reference()
From: Junio C Hamano @ 2018-12-09 1:49 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, stolee, peff
In-Reply-To: <xmqqwooj5xpr.fsf@gitster-ct.c.googlers.com>
Junio C Hamano <gitster@pobox.com> writes:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> When fetching into a repository, a connectivity check is first made by
>> check_exist_and_connected() in builtin/fetch.c that runs:
>> ...
>> Another way to accomplish this effect would be to modify parse_object()
>> to use the commit graph if possible; however, I did not want to change
>> parse_object()'s current behavior of always checking the object
>> signature of the returned object.
>
> Sounds good.
>
>> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
>> ---
>> This patch is now on master.
>
> OK.
>
> Obviously that won't apply to the base for v1 without conflicts, and
> it of course applies cleanly on 'master', but the result of doing so
> will cause the same conflicts when sb/more-repo-in-api is merged on
> top, which means that the same conflicts need to be resolved if this
> wants to be merged to 'next' (or 'pu', FWIW).
So,... as I had to do the reverse rebase anyway, here is the
difference since the previous round, which I came up with by
comparing these two:
(A) merge 'sb/more-repo-in-api' to 'master' and then merge v1 of
this topic to the result.
(B) apply your patch to 'master', and then merge
'sb/more-repo-in-api' to the result.
diff --git a/commit-graph.c b/commit-graph.c
index f78a8e96b5..74a17789f8 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -286,7 +286,8 @@ void close_commit_graph(struct repository *r)
r->objects->commit_graph = NULL;
}
-static int bsearch_graph(struct commit_graph *g, struct object_id *oid, uint32_t *pos)
+static int bsearch_graph(struct commit_graph *g, const struct object_id *oid,
+ uint32_t *pos)
{
return bsearch_hash(oid->hash, g->chunk_oid_fanout,
g->chunk_oid_lookup, g->hash_len, pos);
@@ -377,26 +378,41 @@ static int find_commit_in_graph(struct commit *item, struct commit_graph *g, uin
}
}
-static int parse_commit_in_graph_one(struct repository *r,
- struct commit_graph *g,
- struct commit *item)
+static struct commit *parse_commit_in_graph_one(struct repository *r,
+ struct commit_graph *g,
+ struct commit *shell,
+ const struct object_id *oid)
{
uint32_t pos;
- if (item->object.parsed)
- return 1;
+ if (shell && shell->object.parsed)
+ return shell;
- if (find_commit_in_graph(item, g, &pos))
- return fill_commit_in_graph(r, item, g, pos);
+ if (shell && shell->graph_pos != COMMIT_NOT_FROM_GRAPH) {
+ pos = shell->graph_pos;
+ } else if (bsearch_graph(g, shell ? &shell->object.oid : oid, &pos)) {
+ /* bsearch_graph sets pos */
+ } else {
+ return NULL;
+ }
- return 0;
+ if (!shell) {
+ shell = lookup_commit(r, oid);
+ if (!shell)
+ return NULL;
+ }
+
+ fill_commit_in_graph(r, shell, g, pos);
+ return shell;
}
-int parse_commit_in_graph(struct repository *r, struct commit *item)
+struct commit *parse_commit_in_graph(struct repository *r, struct commit *shell,
+ const struct object_id *oid)
{
if (!prepare_commit_graph(r))
return 0;
- return parse_commit_in_graph_one(r, r->objects->commit_graph, item);
+ return parse_commit_in_graph_one(r, r->objects->commit_graph, shell,
+ oid);
}
void load_commit_graph_info(struct repository *r, struct commit *item)
@@ -1033,7 +1049,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
}
graph_commit = lookup_commit(r, &cur_oid);
- if (!parse_commit_in_graph_one(r, g, graph_commit))
+ if (!parse_commit_in_graph_one(r, g, graph_commit, NULL))
graph_report("failed to parse %s from commit-graph",
oid_to_hex(&cur_oid));
}
diff --git a/commit-graph.h b/commit-graph.h
index 9db40b4d3a..8b7b5985dc 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -13,16 +13,20 @@ struct commit;
char *get_commit_graph_filename(const char *obj_dir);
/*
- * Given a commit struct, try to fill the commit struct info, including:
+ * If the given commit (identified by shell->object.oid or oid) is in the
+ * commit graph, returns a commit struct (reusing shell if it is not NULL)
+ * including the following info:
* 1. tree object
* 2. date
* 3. parents.
*
- * Returns 1 if and only if the commit was found in the packed graph.
+ * If not, returns NULL. See parse_commit_buffer() for the fallback after this
+ * call.
*
- * See parse_commit_buffer() for the fallback after this call.
+ * Either shell or oid must be non-NULL. If both are non-NULL, oid is ignored.
*/
-int parse_commit_in_graph(struct repository *r, struct commit *item);
+struct commit *parse_commit_in_graph(struct repository *r, struct commit *shell,
+ const struct object_id *oid);
/*
* It is possible that we loaded commit contents from the commit buffer,
diff --git a/commit.c b/commit.c
index a5333c7ac6..da7a1d3262 100644
--- a/commit.c
+++ b/commit.c
@@ -462,7 +462,7 @@ int repo_parse_commit_internal(struct repository *r,
return -1;
if (item->object.parsed)
return 0;
- if (use_commit_graph && parse_commit_in_graph(r, item))
+ if (use_commit_graph && parse_commit_in_graph(r, item, NULL))
return 0;
buffer = repo_read_object_file(r, &item->object.oid, &type, &size);
if (!buffer)
diff --git a/revision.c b/revision.c
index 22aa109c14..05fddb5880 100644
--- a/revision.c
+++ b/revision.c
@@ -213,19 +213,9 @@ static struct object *get_reference(struct rev_info *revs, const char *name,
{
struct object *object;
- /*
- * If the repository has commit graphs, repo_parse_commit() avoids
- * reading the object buffer, so use it whenever possible.
- */
- if (oid_object_info(revs->repo, oid, NULL) == OBJ_COMMIT) {
- struct commit *c = lookup_commit(revs->repo, oid);
- if (!repo_parse_commit(revs->repo, c))
- object = (struct object *) c;
- else
- object = NULL;
- } else {
+ object = (struct object *) parse_commit_in_graph(revs->repo, NULL, oid);
+ if (!object)
object = parse_object(revs->repo, oid);
- }
if (!object) {
if (revs->ignore_missing)
diff --git a/t/helper/test-repository.c b/t/helper/test-repository.c
index f7f8618445..689a0b652e 100644
--- a/t/helper/test-repository.c
+++ b/t/helper/test-repository.c
@@ -27,7 +27,7 @@ static void test_parse_commit_in_graph(const char *gitdir, const char *worktree,
c = lookup_commit(&r, commit_oid);
- if (!parse_commit_in_graph(&r, c))
+ if (!parse_commit_in_graph(&r, c, NULL))
die("Couldn't parse commit");
printf("%"PRItime, c->date);
@@ -62,7 +62,7 @@ static void test_get_commit_tree_in_graph(const char *gitdir,
* get_commit_tree_in_graph does not automatically parse the commit, so
* parse it first.
*/
- if (!parse_commit_in_graph(&r, c))
+ if (!parse_commit_in_graph(&r, c, NULL))
die("Couldn't parse commit");
tree = get_commit_tree_in_graph(&r, c);
if (!tree)
^ permalink raw reply related
* Re: why doesn't "git reset" mention optional pathspec?
From: Junio C Hamano @ 2018-12-09 1:42 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Robert P. J. Day, Git Mailing List
In-Reply-To: <CACsJy8A_vaVUt389O5ABa+vsrVDgo1L3WZzVx+P0qfiaY9=p1w@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> Without --mixed, you're using the first form
>
> git reset [-q] [<tree-ish>] [--] <paths>...
>
> which accepts pathspec. If it's not clear, of course patches are welcome.
Yup. The deprecation is about spelling with "--mixed" when invoking
the "restore these paths out of tree-ish (or HEAD when omitted)
only in the index" mode. The feature is of course not deprecated
(but it might have been better if it were "git checkout --cached").
^ permalink raw reply
* Re: [PATCH v3 1/1] git clone <url> C:\cygwin\home\USER\repo' is working (again)
From: Junio C Hamano @ 2018-12-09 1:39 UTC (permalink / raw)
To: tboegi; +Cc: git, svnpenn, johannes.schindelin
In-Reply-To: <20181208151109.2097-1-tboegi@web.de>
tboegi@web.de writes:
> - The "DOS" moniker is still used for 2 reasons:
> Windows inherited the "drive letter" concept from DOS,
> and everybody (tm) familar with the code and the path handling
> in Git is used to that wording.
Yeah, for the same reason as win32 can refer to their API that is
used on platforms that are 64-bit, the fact that the "drive letter"
concept came from DOS is so widely ingrained, I do not think it is a
beter change to deviate from it.
> And, before any cleanup is done, I sould like to ask if anybody
> can build the code with VS and confirm that it works, please ?
Yup, that is much more important.
Thanks.
^ permalink raw reply
* Re: [PATCH] terminology tweak: prune -> path limiting
From: Junio C Hamano @ 2018-12-09 1:36 UTC (permalink / raw)
To: Matthew DeVore; +Cc: git, jrn, matvore, dstolee, pclouds, peff
In-Reply-To: <20181206213315.64423-1-matvore@google.com>
Matthew DeVore <matvore@google.com> writes:
> In the codebase, "prune" is a highly overloaded term, and it caused me a
> lot of trouble to figure out what it meant when it was used in the
> context of path limiting. Stop using the word "prune" when we really
> mean "path limiting."
path limiting is also used for two purposes. "pruning", which is to
cull the side branches that do not contribute the changes made to
the paths we are interested in, and showing only the changes to the
paths that match pathspec.
AFAIK, "prune" is also used to describe unreachable loose objects,
but that use is fairly isolated and have little risk of being
confusing too much. Are there other uses to make you consider it
"highly overloaded"?
My gut feeling is that the result is not reducing "overloading" in a
meaningful way, and this change is not worth the churn, but it
depends on the answer to the above question.
Thanks.
^ permalink raw reply
* Re: Bug: git add --patch does not honor "diff.noprefix"
From: Junio C Hamano @ 2018-12-09 1:30 UTC (permalink / raw)
To: Christian Weiske; +Cc: git
In-Reply-To: <c165fc5f-c452-fb2e-8ac3-d2afb12948bc@cweiske.de>
Christian Weiske <cweiske@cweiske.de> writes:
> When running "git add -p" on git version 2.19.2 and "diff.noprefix" set
> to true, it still shows the "a/" and "b/" prefixes.
>
> This issue has been reported in 2016 already[1], but is still there in
> 2.19.2.
It is very likely because it is a non-issue.
The reason why prefixes are customizable is to match the convention
used to show your patch to others, but the patch to be immediately
consumed within an "add -i/-p" session is viewed only by the user,
so it is much much lower priority to change the code. I guess the
reason why no such change was made is because nobody felt it worth
the trouble to change the code to use a non-standard prefix when
producing the patch to be shown, and then also change the code to
accept a non-standard prefix when using the chosen patch to be
applied.
^ permalink raw reply
* Re: [PATCH on master v2] revision: use commit graph in get_reference()
From: Junio C Hamano @ 2018-12-09 0:51 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, stolee, peff
In-Reply-To: <20181207215034.213211-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> When fetching into a repository, a connectivity check is first made by
> check_exist_and_connected() in builtin/fetch.c that runs:
> ...
> Another way to accomplish this effect would be to modify parse_object()
> to use the commit graph if possible; however, I did not want to change
> parse_object()'s current behavior of always checking the object
> signature of the returned object.
Sounds good.
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> This patch is now on master.
OK.
Obviously that won't apply to the base for v1 without conflicts, and
it of course applies cleanly on 'master', but the result of doing so
will cause the same conflicts when sb/more-repo-in-api is merged on
top, which means that the same conflicts need to be resolved if this
wants to be merged to 'next' (or 'pu', FWIW).
> diff --git a/commit-graph.c b/commit-graph.c
> index 40c855f185..a571b523b7 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -374,24 +375,41 @@ static int find_commit_in_graph(struct commit *item, struct commit_graph *g, uin
> }
> }
>
> -static int parse_commit_in_graph_one(struct commit_graph *g, struct commit *item)
> +static struct commit *parse_commit_in_graph_one(struct repository *r,
> + struct commit_graph *g,
> + struct commit *shell,
> + const struct object_id *oid)
Now the complexity of the behaviour of this function deserves to be
documented in a comment in front. Let me see if I can get it
correctly without such a comment by explaining the function aloud.
The caller may or may not have already obtained an in-core commit
object for a given object name, so shell could be NULL but otherwise
it could be used for optimization. When shell==NULL, the function
looks up the commit object using the oid parameter instead. The
returned in-core commit has the parents etc. filled as if we ran
parse_commit() on it. If the commit is not yet in the graph, the
caller may get a NULL even if the commit exists.
> {
> uint32_t pos;
>
> - if (item->object.parsed)
> - return 1;
> + if (shell && shell->object.parsed)
> + return shell;
>
> - if (find_commit_in_graph(item, g, &pos))
> - return fill_commit_in_graph(item, g, pos);
> + if (shell && shell->graph_pos != COMMIT_NOT_FROM_GRAPH) {
> + pos = shell->graph_pos;
> + } else if (bsearch_graph(g, shell ? &shell->object.oid : oid, &pos)) {
> + /* bsearch_graph sets pos */
Please spell an empty statement like so:
; /* comment */
> + } else {
> + return NULL;
We come here when the commit (either came from shell or from oid) is
not found by bsearch_graph(). "Is the caller prepared for it, and
how?" is a natural question a reader would have. Let's read on.
> + }
>
> - return 0;
> + if (!shell) {
> + shell = lookup_commit(r, oid);
> + if (!shell)
> + return NULL;
> + }
> +
> + fill_commit_in_graph(shell, g, pos);
> + return shell;
> }
>
> -int parse_commit_in_graph(struct repository *r, struct commit *item)
> +struct commit *parse_commit_in_graph(struct repository *r, struct commit *shell,
> + const struct object_id *oid)
> {
> if (!prepare_commit_graph(r))
> return 0;
> - return parse_commit_in_graph_one(r->objects->commit_graph, item);
> + return parse_commit_in_graph_one(r, r->objects->commit_graph, shell,
> + oid);
> }
>
> void load_commit_graph_info(struct repository *r, struct commit *item)
> @@ -1025,7 +1043,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g)
> }
>
> graph_commit = lookup_commit(r, &cur_oid);
> - if (!parse_commit_in_graph_one(g, graph_commit))
> + if (!parse_commit_in_graph_one(r, g, graph_commit, NULL))
> graph_report("failed to parse %s from commit-graph",
> oid_to_hex(&cur_oid));
> }
> diff --git a/commit-graph.h b/commit-graph.h
> index 9db40b4d3a..8b7b5985dc 100644
> --- a/commit-graph.h
> +++ b/commit-graph.h
> @@ -13,16 +13,20 @@ struct commit;
> char *get_commit_graph_filename(const char *obj_dir);
>
> /*
> - * Given a commit struct, try to fill the commit struct info, including:
> + * If the given commit (identified by shell->object.oid or oid) is in the
> + * commit graph, returns a commit struct (reusing shell if it is not NULL)
> + * including the following info:
> * 1. tree object
> * 2. date
> * 3. parents.
> *
> - * Returns 1 if and only if the commit was found in the packed graph.
> + * If not, returns NULL. See parse_commit_buffer() for the fallback after this
> + * call.
> *
> - * See parse_commit_buffer() for the fallback after this call.
> + * Either shell or oid must be non-NULL. If both are non-NULL, oid is ignored.
> */
OK, the eventual caller is the caller of this thing, which should
have been prepared to see NULL for a commit that is too new. So
that should be OK.
> -int parse_commit_in_graph(struct repository *r, struct commit *item);
> +struct commit *parse_commit_in_graph(struct repository *r, struct commit *shell,
> + const struct object_id *oid);
>
> /*
> * It is possible that we loaded commit contents from the commit buffer,
> diff --git a/commit.c b/commit.c
> index d13a7bc374..88eb580c5a 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -456,7 +456,7 @@ int parse_commit_internal(struct commit *item, int quiet_on_missing, int use_com
> return -1;
> if (item->object.parsed)
> return 0;
> - if (use_commit_graph && parse_commit_in_graph(the_repository, item))
> + if (use_commit_graph && parse_commit_in_graph(the_repository, item, NULL))
> return 0;
> buffer = read_object_file(&item->object.oid, &type, &size);
> if (!buffer)
> diff --git a/revision.c b/revision.c
> index 13e0519c02..05fddb5880 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -213,7 +213,10 @@ static struct object *get_reference(struct rev_info *revs, const char *name,
> {
> struct object *object;
>
> - object = parse_object(revs->repo, oid);
> + object = (struct object *) parse_commit_in_graph(revs->repo, NULL, oid);
> + if (!object)
> + object = parse_object(revs->repo, oid);
OK and this is such a caller. I think a general rule of thumb is
that we need to access recent history a lot more often than the
older part of the history, and having to fall back for more recent
commits feels a bit disturbing, but I do not see an easy way to
reverse the performance characteristics offhand.
^ permalink raw reply
* Re: [PATCH] git-rebase.txt: update note about directory rename detection and am
From: Junio C Hamano @ 2018-12-09 0:22 UTC (permalink / raw)
To: Elijah Newren; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <CABPp-BGSmyAb_d52TLS=7oQyMYxt=EYjNdDofY4nzLd9CYvwuQ@mail.gmail.com>
Elijah Newren <newren@gmail.com> writes:
> On Fri, Dec 7, 2018 at 9:51 AM Johannes Sixt <j6t@kdbg.org> wrote:
>>
>> From: Elijah Newren <newren@gmail.com>
>>
>> In commit 6aba117d5cf7 ("am: avoid directory rename detection when
>> calling recursive merge machinery", 2018-08-29), the git-rebase manpage
>> probably should have also been updated to note the stronger
>> incompatibility between git-am and directory rename detection. Update
>> it now.
>>
>> Signed-off-by: Elijah Newren <newren@gmail.com>
>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>> ---
>> Documentation/git-rebase.txt | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
>> index 41631df6e4..7bea21e8e3 100644
>> --- a/Documentation/git-rebase.txt
>> +++ b/Documentation/git-rebase.txt
>> @@ -569,8 +569,9 @@ it to keep commits that started empty.
>> Directory rename detection
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> -The merge and interactive backends work fine with
>> -directory rename detection. The am backend sometimes does not.
>> +Directory rename heuristics are enabled in the merge and interactive
>> +backends. Due to the lack of accurate tree information, directory
>> +rename detection is disabled in the am backend.
>>
>> include::merge-strategies.txt[]
>
> I was intending to send this out the past couple days, was just kinda
> busy. Thanks for handling it for me.
Thanks, both. I can live with format=flowed, but would appreciate
if we can avoid it next time.
^ permalink raw reply
* Re: [WIP RFC 1/5] Documentation: order protocol v2 sections
From: Junio C Hamano @ 2018-12-09 0:15 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git
In-Reply-To: <20181206225431.135449-1-jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
>> > The git command line expects Git servers to follow a specific order of
>>
>> "Command line"? It sounds like you are talking about the order of
>> command line arguments and options, but apparently that is not what
>> you are doing. Is it "The git over-the-wire protocol"?
>
> I meant to say the current Git implementation, as opposed to what is
> written in the specification. I'll replace it with "The current C Git
> implementation".
Yeah, that would avoid confusing future readers; sounds good.
>> Earlier, we said that shallow-info is not given when packfile is not
>> there. That is captured in the updated EBNF above. We don't have a
>> corresponding removal of a bullet point for wanted-refs section below
>> but probably that is because the original did not have corresponding
>> bullet point to begin with.
>
> That's because the corresponding bullet point had other information.
> Quoted in full below:
>
>> * This section is only included if the client has requested a
>> ref using a 'want-ref' line and if a packfile section is also
>> included in the response.
>
> I could reword it to "If a packfile section is included in the response,
> this section is only included if the client has requested a ref using a
> 'want-ref' line", but I don't think that is significantly clearer.
I don't either. I didn't mean to suggest to change anything in this
part. I was just giving an observation---two parallel things do not
get updates in tandem, and that is because they were not described
the same way to begin with, which was a good enough explanation.
^ permalink raw reply
* Re: [PATCH 1/4] submodule update: add regression test with old style setups
From: Junio C Hamano @ 2018-12-09 0:11 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <20181207235425.128568-2-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> As f178c13fda (Revert "Merge branch 'sb/submodule-core-worktree'",
> 2018-09-07) was produced shortly before a release, nobody asked for
> a regression test to be included. Add a regression test that makes sure
> that the invocation of `git submodule update` on old setups doesn't
> produce errors as pointed out in f178c13fda.
>
> The place to add such a regression test may look odd in t7412, but
> that is the best place as there we setup old style submodule setups
> explicitly.
Very good first step. Thanks.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> t/t7412-submodule-absorbgitdirs.sh | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
> index ce74c12da2..1cfa150768 100755
> --- a/t/t7412-submodule-absorbgitdirs.sh
> +++ b/t/t7412-submodule-absorbgitdirs.sh
> @@ -75,7 +75,12 @@ test_expect_success 're-setup nested submodule' '
> GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
> core.worktree "../../../nested" &&
> # make sure this re-setup is correct
> - git status --ignore-submodules=none
> + git status --ignore-submodules=none &&
> +
> + # also make sure this old setup does not regress
> + git submodule update --init --recursive >out 2>err &&
> + test_must_be_empty out &&
> + test_must_be_empty err
> '
>
> test_expect_success 'absorb the git dir in a nested submodule' '
^ permalink raw reply
* Re: Retrieving a file in git that was deleted and committed
From: biswaranjan panda @ 2018-12-09 0:07 UTC (permalink / raw)
To: Jeff King; +Cc: Bryan Turner, git
In-Reply-To: <20181208072915.GA20697@sigill.intra.peff.net>
>You can feed a set of revisions to git-blame with the "-S" option, but I
>don't offhand know how it handles diffs (I think it would have to still
>diff each commit against its parent, since history is non-linear, and a
>list is inherently linear). You might want to experiment with that.
>Other than that, you can play with git-replace to produce a fake
>history, as if the deletion never happened. But note that will affect
>all commands, not just one particular blame. It might be a neat way to
>play with blame, but I doubt I'd leave the replacement in place in the
>long term.
> -Peff
Ah I see. Will try git-replace. Thanks!
On Fri, Dec 7, 2018 at 11:29 PM Jeff King <peff@peff.net> wrote:
>
> On Fri, Dec 07, 2018 at 01:50:57PM -0800, biswaranjan panda wrote:
>
> > Thanks Jeff and Bryan! However, I am curious that if there were a way
> > to tell git blame to skip a commit (the one which added the file again
> > and maybe the one which deleted it originally) while it walks back
> > through history, then it should just get back the
> > entire history right ?
>
> Not easily. ;)
>
> You can feed a set of revisions to git-blame with the "-S" option, but I
> don't offhand know how it handles diffs (I think it would have to still
> diff each commit against its parent, since history is non-linear, and a
> list is inherently linear). You might want to experiment with that.
>
> Other than that, you can play with git-replace to produce a fake
> history, as if the deletion never happened. But note that will affect
> all commands, not just one particular blame. It might be a neat way to
> play with blame, but I doubt I'd leave the replacement in place in the
> long term.
>
> -Peff
--
Thanks,
-Biswa
^ permalink raw reply
* [PATCH] rebase docs: drop stray word in merge command description
From: Kyle Meyer @ 2018-12-08 23:15 UTC (permalink / raw)
To: git; +Cc: Johannes.Schindelin, Kyle Meyer
Delete a misplaced word introduced by caafecfcf1 (rebase
--rebase-merges: adjust man page for octopus support, 2018-03-09).
Signed-off-by: Kyle Meyer <kyle@kyleam.com>
---
Documentation/git-rebase.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index dff17b3178..2ee535fb23 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -979,7 +979,7 @@ when the merge operation did not even start), it is rescheduled immediately.
At this time, the `merge` command will *always* use the `recursive`
merge strategy for regular merges, and `octopus` for octopus merges,
-strategy, with no way to choose a different one. To work around
+with no way to choose a different one. To work around
this, an `exec` command can be used to call `git merge` explicitly,
using the fact that the labels are worktree-local refs (the ref
`refs/rewritten/onto` would correspond to the label `onto`, for example).
--
2.19.2
^ permalink raw reply related
* Re: why doesn't "git reset" mention optional pathspec?
From: Duy Nguyen @ 2018-12-08 18:17 UTC (permalink / raw)
To: Robert P. J. Day; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.21.1812081236220.32716@localhost.localdomain>
On Sat, Dec 8, 2018 at 6:37 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
>
> On Sat, 8 Dec 2018, Duy Nguyen wrote:
>
> > On Sat, Dec 8, 2018 at 6:32 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> > >
> > > On Sat, 8 Dec 2018, Duy Nguyen wrote:
> > >
> > > > On Sat, Dec 8, 2018 at 5:08 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> > > > >
> > > > >
> > > > > from "man git-reset":
> > > > >
> > > > > SYNOPSIS
> > > > > git reset [-q] [<tree-ish>] [--] <paths>...
> > > > > git reset (--patch | -p) [<tree-ish>] [--] [<paths>...]
> > > > > git reset [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
> > > > >
> > > > > oddly, the third form says nothing about possible "<paths>", even
> > > > > though i'm pretty sure they're valid in that third case (at least
> > > > > for "--mixed"). thoughts? is that just an oversight in the man
> > > > > page?
> > > >
> > > > --mixed prints a deprecation warning. I don't think it's worth
> > > > making the synopsis more complicated for that. All other modes
> > > > reject pathspec.
> > >
> > > i just tested this, and i don't see a deprecation warning.
> >
> > Hmm.. maybe I misread the code. I just tried it
> >
> > $ ./git reset --mixed HEAD foo
> > warning: --mixed with paths is deprecated; use 'git reset -- <paths>' instead.
>
> weird ... i just tried this two ways, explicitly specifying
> "--mixed" and also without (which is the default mode, right?), and i
> got the deprecated message with the first but not the second. that
> seems ... odd.
Without --mixed, you're using the first form
git reset [-q] [<tree-ish>] [--] <paths>...
which accepts pathspec. If it's not clear, of course patches are welcome.
--
Duy
^ permalink raw reply
* Re: why doesn't "git reset" mention optional pathspec?
From: Robert P. J. Day @ 2018-12-08 17:42 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8AC-anZ=EA3zxWeX8UUNcZiKsQMu8x0eCHAOCUjFWoFuQ@mail.gmail.com>
On Sat, 8 Dec 2018, Duy Nguyen wrote:
> On Sat, Dec 8, 2018 at 6:32 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> >
> > On Sat, 8 Dec 2018, Duy Nguyen wrote:
> >
> > > On Sat, Dec 8, 2018 at 5:08 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> > > >
> > > >
> > > > from "man git-reset":
> > > >
> > > > SYNOPSIS
> > > > git reset [-q] [<tree-ish>] [--] <paths>...
> > > > git reset (--patch | -p) [<tree-ish>] [--] [<paths>...]
> > > > git reset [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
> > > >
> > > > oddly, the third form says nothing about possible "<paths>", even
> > > > though i'm pretty sure they're valid in that third case (at least
> > > > for "--mixed"). thoughts? is that just an oversight in the man
> > > > page?
> > >
> > > --mixed prints a deprecation warning. I don't think it's worth
> > > making the synopsis more complicated for that. All other modes
> > > reject pathspec.
> >
> > i just tested this, and i don't see a deprecation warning.
>
> Hmm.. maybe I misread the code. I just tried it
>
> $ ./git reset --mixed HEAD foo
> warning: --mixed with paths is deprecated; use 'git reset -- <paths>' instead.
my test:
Changes to be committed:
(use "git reset HEAD <file>..." to unstage)
modified: README.asc
modified: Rakefile
$ git reset -- README.asc
Unstaged changes after reset:
M README.asc
$ git reset --mixed -- Rakefile
warning: --mixed with paths is deprecated; use 'git reset -- <paths>' instead.
Unstaged changes after reset:
M README.asc
M Rakefile
$
that definitely seems inconsistent.
rday
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca/dokuwiki
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
^ permalink raw reply
* Re: why doesn't "git reset" mention optional pathspec?
From: Robert P. J. Day @ 2018-12-08 17:37 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8AC-anZ=EA3zxWeX8UUNcZiKsQMu8x0eCHAOCUjFWoFuQ@mail.gmail.com>
On Sat, 8 Dec 2018, Duy Nguyen wrote:
> On Sat, Dec 8, 2018 at 6:32 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> >
> > On Sat, 8 Dec 2018, Duy Nguyen wrote:
> >
> > > On Sat, Dec 8, 2018 at 5:08 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> > > >
> > > >
> > > > from "man git-reset":
> > > >
> > > > SYNOPSIS
> > > > git reset [-q] [<tree-ish>] [--] <paths>...
> > > > git reset (--patch | -p) [<tree-ish>] [--] [<paths>...]
> > > > git reset [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
> > > >
> > > > oddly, the third form says nothing about possible "<paths>", even
> > > > though i'm pretty sure they're valid in that third case (at least
> > > > for "--mixed"). thoughts? is that just an oversight in the man
> > > > page?
> > >
> > > --mixed prints a deprecation warning. I don't think it's worth
> > > making the synopsis more complicated for that. All other modes
> > > reject pathspec.
> >
> > i just tested this, and i don't see a deprecation warning.
>
> Hmm.. maybe I misread the code. I just tried it
>
> $ ./git reset --mixed HEAD foo
> warning: --mixed with paths is deprecated; use 'git reset -- <paths>' instead.
weird ... i just tried this two ways, explicitly specifying
"--mixed" and also without (which is the default mode, right?), and i
got the deprecated message with the first but not the second. that
seems ... odd.
rday
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca/dokuwiki
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
^ permalink raw reply
* Re: why doesn't "git reset" mention optional pathspec?
From: Duy Nguyen @ 2018-12-08 17:34 UTC (permalink / raw)
To: Robert P. J. Day; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.21.1812081232240.32380@localhost.localdomain>
On Sat, Dec 8, 2018 at 6:32 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
>
> On Sat, 8 Dec 2018, Duy Nguyen wrote:
>
> > On Sat, Dec 8, 2018 at 5:08 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> > >
> > >
> > > from "man git-reset":
> > >
> > > SYNOPSIS
> > > git reset [-q] [<tree-ish>] [--] <paths>...
> > > git reset (--patch | -p) [<tree-ish>] [--] [<paths>...]
> > > git reset [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
> > >
> > > oddly, the third form says nothing about possible "<paths>", even
> > > though i'm pretty sure they're valid in that third case (at least
> > > for "--mixed"). thoughts? is that just an oversight in the man
> > > page?
> >
> > --mixed prints a deprecation warning. I don't think it's worth
> > making the synopsis more complicated for that. All other modes
> > reject pathspec.
>
> i just tested this, and i don't see a deprecation warning.
Hmm.. maybe I misread the code. I just tried it
$ ./git reset --mixed HEAD foo
warning: --mixed with paths is deprecated; use 'git reset -- <paths>' instead.
--
Duy
^ permalink raw reply
* Re: why doesn't "git reset" mention optional pathspec?
From: Robert P. J. Day @ 2018-12-08 17:32 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8APyyAWM+L=HU1XM4V+qdTWqjto6x=Q06By8DbgtYfpCA@mail.gmail.com>
On Sat, 8 Dec 2018, Duy Nguyen wrote:
> On Sat, Dec 8, 2018 at 5:08 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> >
> >
> > from "man git-reset":
> >
> > SYNOPSIS
> > git reset [-q] [<tree-ish>] [--] <paths>...
> > git reset (--patch | -p) [<tree-ish>] [--] [<paths>...]
> > git reset [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
> >
> > oddly, the third form says nothing about possible "<paths>", even
> > though i'm pretty sure they're valid in that third case (at least
> > for "--mixed"). thoughts? is that just an oversight in the man
> > page?
>
> --mixed prints a deprecation warning. I don't think it's worth
> making the synopsis more complicated for that. All other modes
> reject pathspec.
i just tested this, and i don't see a deprecation warning.
rday
--
========================================================================
Robert P. J. Day Ottawa, Ontario, CANADA
http://crashcourse.ca/dokuwiki
Twitter: http://twitter.com/rpjday
LinkedIn: http://ca.linkedin.com/in/rpjday
========================================================================
^ permalink raw reply
* Re: why doesn't "git reset" mention optional pathspec?
From: Duy Nguyen @ 2018-12-08 17:28 UTC (permalink / raw)
To: Robert P. J. Day; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.2.21.1812081103500.29142@localhost.localdomain>
On Sat, Dec 8, 2018 at 5:08 PM Robert P. J. Day <rpjday@crashcourse.ca> wrote:
>
>
> from "man git-reset":
>
> SYNOPSIS
> git reset [-q] [<tree-ish>] [--] <paths>...
> git reset (--patch | -p) [<tree-ish>] [--] [<paths>...]
> git reset [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
>
> oddly, the third form says nothing about possible "<paths>", even
> though i'm pretty sure they're valid in that third case (at least for
> "--mixed"). thoughts? is that just an oversight in the man page?
--mixed prints a deprecation warning. I don't think it's worth making
the synopsis more complicated for that. All other modes reject
pathspec.
--
Duy
^ permalink raw reply
* [PATCH v4 4/7] pretty: allow showing specific trailers
From: Anders Waldenborg @ 2018-12-08 16:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Olga Telezhnaya, Anders Waldenborg
In-Reply-To: <20181208163647.19538-1-anders@0x63.nu>
Adds a new "key=X" option to "%(trailers)" which will cause it to only
print trailer lines which match any of the specified keys.
Signed-off-by: Anders Waldenborg <anders@0x63.nu>
---
Documentation/pretty-formats.txt | 8 +++++
pretty.c | 47 ++++++++++++++++++++++++++---
t/t4205-log-pretty-formats.sh | 51 ++++++++++++++++++++++++++++++++
trailer.c | 10 ++++---
trailer.h | 2 ++
5 files changed, 110 insertions(+), 8 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d33b072eb2..d6add831c0 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -225,6 +225,14 @@ endif::git-rev-list[]
linkgit:git-interpret-trailers[1]. The
`trailers` string may be followed by a colon
and zero or more comma-separated options:
+** 'key=<K>': only show trailers with specified key. Matching is done
+ case-insensitively and trailing colon is optional. If option is
+ given multiple times trailer lines matching any of the keys are
+ shown. This option automatically enables the `only` option so that
+ non-trailer lines in the trailer block are hidden. If that is not
+ desired it can be disabled with `only=false`. E.g.,
+ `%(trailers:key=Reviewed-by)` shows trailer lines with key
+ `Reviewed-by`.
** 'only[=val]': select whether non-trailer lines from the trailer
block should be included. The `only` keyword may optionally be
followed by an equal sign and one of `true`, `on`, `yes` to omit or
diff --git a/pretty.c b/pretty.c
index 044447e6c0..541a553ccc 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1056,13 +1056,20 @@ static size_t parse_padding_placeholder(struct strbuf *sb,
return 0;
}
-static int match_placeholder_arg(const char *to_parse, const char *candidate,
- const char **end)
+static int match_placeholder_arg_value(const char *to_parse, const char *candidate,
+ const char **end, const char **valuestart, size_t *valuelen)
{
const char *p;
if (!(skip_prefix(to_parse, candidate, &p)))
return 0;
+ if (valuestart) {
+ if (*p != '=')
+ return 0;
+ *valuestart = p + 1;
+ *valuelen = strcspn(*valuestart, ",)");
+ p = *valuestart + *valuelen;
+ }
if (*p == ',') {
*end = p + 1;
return 1;
@@ -1074,6 +1081,12 @@ static int match_placeholder_arg(const char *to_parse, const char *candidate,
return 0;
}
+static int match_placeholder_arg(const char *to_parse, const char *candidate,
+ const char **end)
+{
+ return match_placeholder_arg_value(to_parse, candidate, end, NULL, NULL);
+}
+
static int match_placeholder_bool_arg(const char *to_parse, const char *candidate,
const char **end, int *val)
{
@@ -1095,7 +1108,19 @@ static int match_placeholder_bool_arg(const char *to_parse, const char *candidat
*val = 1;
return 1;
}
+ return 0;
+}
+static int format_trailer_match_cb(const struct strbuf *key, void *ud)
+{
+ const struct string_list *list = ud;
+ const struct string_list_item *item;
+
+ for_each_string_list_item (item, list) {
+ if (key->len == (uintptr_t)item->util &&
+ !strncasecmp (item->string, key->buf, key->len))
+ return 1;
+ }
return 0;
}
@@ -1337,6 +1362,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
+ struct string_list filter_list = STRING_LIST_INIT_NODUP;
size_t ret = 0;
opts.no_divider = 1;
@@ -1344,8 +1370,20 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
if (*arg == ':') {
arg++;
for (;;) {
- if (!match_placeholder_bool_arg(arg, "only", &arg, &opts.only_trailers) &&
- !match_placeholder_bool_arg(arg, "unfold", &arg, &opts.unfold))
+ const char *argval;
+ size_t arglen;
+
+ if (match_placeholder_arg_value(arg, "key", &arg, &argval, &arglen)) {
+ uintptr_t len = arglen;
+ if (len && argval[len - 1] == ':')
+ len--;
+ string_list_append(&filter_list, argval)->util = (char *)len;
+
+ opts.filter = format_trailer_match_cb;
+ opts.filter_data = &filter_list;
+ opts.only_trailers = 1;
+ } else if (!match_placeholder_bool_arg(arg, "only", &arg, &opts.only_trailers) &&
+ !match_placeholder_bool_arg(arg, "unfold", &arg, &opts.unfold))
break;
}
}
@@ -1353,6 +1391,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
format_trailers_from_commit(sb, msg + c->subject_off, &opts);
ret = arg - placeholder + 1;
}
+ string_list_clear (&filter_list, 0);
return ret;
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 63730a4ec0..54239290cf 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -616,6 +616,57 @@ test_expect_success ':only and :unfold work together' '
test_cmp expect actual
'
+test_expect_success 'pretty format %(trailers:key=foo) shows that trailer' '
+ git log --no-walk --pretty="format:%(trailers:key=Acked-by)" >actual &&
+ echo "Acked-by: A U Thor <author@example.com>" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'pretty format %(trailers:key=foo) is case insensitive' '
+ git log --no-walk --pretty="format:%(trailers:key=AcKed-bY)" >actual &&
+ echo "Acked-by: A U Thor <author@example.com>" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'pretty format %(trailers:key=foo:) trailing colon also works' '
+ git log --no-walk --pretty="format:%(trailers:key=Acked-by:)" >actual &&
+ echo "Acked-by: A U Thor <author@example.com>" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'pretty format %(trailers:key=foo) multiple keys' '
+ git log --no-walk --pretty="format:%(trailers:key=Acked-by:,key=Signed-off-By)" >actual &&
+ grep -v patch.description <trailers >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailers:key=nonexistant) becomes empty' '
+ git log --no-walk --pretty="x%(trailers:key=Nacked-by)x" >actual &&
+ echo "xx" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailers:key=foo) handles multiple lines even if folded' '
+ git log --no-walk --pretty="format:%(trailers:key=Signed-Off-by)" >actual &&
+ grep -v patch.description <trailers | grep -v Acked-by >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailers:key=foo,unfold) properly unfolds' '
+ git log --no-walk --pretty="format:%(trailers:key=Signed-Off-by,unfold)" >actual &&
+ unfold <trailers | grep Signed-off-by >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'pretty format %(trailers:key=foo,only=no) also includes nontrailer lines' '
+ git log --no-walk --pretty="format:%(trailers:key=Acked-by,only=no)" >actual &&
+ {
+ echo "Acked-by: A U Thor <author@example.com>" &&
+ grep patch.description <trailers
+ } >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'trailer parsing not fooled by --- line' '
git commit --allow-empty -F - <<-\EOF &&
this is the subject
diff --git a/trailer.c b/trailer.c
index 0796f326b3..d6da555cd7 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1132,7 +1132,7 @@ static void format_trailer_info(struct strbuf *out,
size_t i;
/* If we want the whole block untouched, we can take the fast path. */
- if (!opts->only_trailers && !opts->unfold) {
+ if (!opts->only_trailers && !opts->unfold && !opts->filter) {
strbuf_add(out, info->trailer_start,
info->trailer_end - info->trailer_start);
return;
@@ -1147,10 +1147,12 @@ static void format_trailer_info(struct strbuf *out,
struct strbuf val = STRBUF_INIT;
parse_trailer(&tok, &val, NULL, trailer, separator_pos);
- if (opts->unfold)
- unfold_value(&val);
+ if (!opts->filter || opts->filter(&tok, opts->filter_data)) {
+ if (opts->unfold)
+ unfold_value(&val);
- strbuf_addf(out, "%s: %s\n", tok.buf, val.buf);
+ strbuf_addf(out, "%s: %s\n", tok.buf, val.buf);
+ }
strbuf_release(&tok);
strbuf_release(&val);
diff --git a/trailer.h b/trailer.h
index b997739649..5255b676de 100644
--- a/trailer.h
+++ b/trailer.h
@@ -72,6 +72,8 @@ struct process_trailer_options {
int only_input;
int unfold;
int no_divider;
+ int (*filter)(const struct strbuf *, void *);
+ void *filter_data;
};
#define PROCESS_TRAILER_OPTIONS_INIT {0}
--
2.17.1
^ permalink raw reply related
* [PATCH v4 7/7] pretty: add support for separator option in %(trailers)
From: Anders Waldenborg @ 2018-12-08 16:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Olga Telezhnaya, Anders Waldenborg
In-Reply-To: <20181208163647.19538-1-anders@0x63.nu>
By default trailer lines are terminated by linebreaks ('\n'). By
specifying the new 'separator' option they will instead be separated by
user provided string and have separator semantics rather than terminator
semantics. The separator string can contain the literal formatting codes
%n and %xNN allowing it to be things that are otherwise hard to type
such as %x00, or comma and end-parenthesis which would break parsing.
E.g:
$ git log --pretty='%(trailers:key=Reviewed-by,valueonly,separator=%x00)'
Signed-off-by: Anders Waldenborg <anders@0x63.nu>
---
Documentation/pretty-formats.txt | 9 ++++++++
pretty.c | 10 +++++++++
t/t4205-log-pretty-formats.sh | 36 ++++++++++++++++++++++++++++++++
trailer.c | 15 +++++++++++--
trailer.h | 1 +
5 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index a920dd15b1..ce087dee80 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -239,6 +239,15 @@ endif::git-rev-list[]
`false`, `off`, `no` to show the non-trailer lines. If option is
given without value it is enabled. If given multiple times the last
value is used.
+** 'separator=<SEP>': specify a separator inserted between trailer
+ lines. When this option is not given each trailer line is
+ terminated with a line feed character. The string SEP may contain
+ the literal formatting codes described above. To use comma as
+ separator one must use `%x2C` as it would otherwise be parsed as
+ next option. If separator option is given multiple times only the
+ last one is used. E.g., `%(trailers:key=Ticket,separator=%x2C )`
+ shows all trailer lines whose key is "Ticket" separated by a comma
+ and a space.
** 'unfold[=val]': make it behave as if interpret-trailer's `--unfold`
option was given. In same way as to for `only` it can be followed
by an equal sign and explicit value. E.g.,
diff --git a/pretty.c b/pretty.c
index 50d0b5830d..c7609493ee 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1357,6 +1357,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
struct string_list filter_list = STRING_LIST_INIT_NODUP;
+ struct strbuf sepbuf = STRBUF_INIT;
size_t ret = 0;
opts.no_divider = 1;
@@ -1376,6 +1377,14 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
opts.filter = format_trailer_match_cb;
opts.filter_data = &filter_list;
opts.only_trailers = 1;
+ } else if (match_placeholder_arg_value(arg, "separator", &arg, &argval, &arglen)) {
+ char *fmt;
+
+ strbuf_reset(&sepbuf);
+ fmt = xstrndup(argval, arglen);
+ strbuf_expand(&sepbuf, fmt, strbuf_expand_literal_cb, NULL);
+ free(fmt);
+ opts.separator = &sepbuf;
} else if (!match_placeholder_bool_arg(arg, "only", &arg, &opts.only_trailers) &&
!match_placeholder_bool_arg(arg, "unfold", &arg, &opts.unfold) &&
!match_placeholder_bool_arg(arg, "valueonly", &arg, &opts.value_only))
@@ -1387,6 +1396,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
ret = arg - placeholder + 1;
}
string_list_clear (&filter_list, 0);
+ strbuf_release(&sepbuf);
return ret;
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 22336c5485..282369dac0 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -673,6 +673,42 @@ test_expect_success '%(trailers:key=foo,valueonly) shows only value' '
test_cmp expect actual
'
+test_expect_success 'pretty format %(trailers:separator) changes separator' '
+ git log --no-walk --pretty=format:"X%(trailers:separator=%x00,unfold)X" >actual &&
+ printf "XSigned-off-by: A U Thor <author@example.com>\0Acked-by: A U Thor <author@example.com>\0[ v2 updated patch description ]\0Signed-off-by: A U Thor <author@example.com>X" >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success 'pretty format %(trailers) combining separator/key/valueonly' '
+ git commit --allow-empty -F - <<-\EOF &&
+ Important fix
+
+ The fix is explained here
+
+ Closes: #1234
+ EOF
+
+ git commit --allow-empty -F - <<-\EOF &&
+ Another fix
+
+ The fix is explained here
+
+ Closes: #567
+ Closes: #890
+ EOF
+
+ git commit --allow-empty -F - <<-\EOF &&
+ Does not close any tickets
+ EOF
+
+ git log --pretty="%s% (trailers:separator=%x2c%x20,key=Closes,valueonly)" HEAD~3.. >actual &&
+ test_write_lines \
+ "Does not close any tickets" \
+ "Another fix #567, #890" \
+ "Important fix #1234" >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'trailer parsing not fooled by --- line' '
git commit --allow-empty -F - <<-\EOF &&
this is the subject
diff --git a/trailer.c b/trailer.c
index d0d9e91631..0c414f2fed 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1129,10 +1129,11 @@ static void format_trailer_info(struct strbuf *out,
const struct trailer_info *info,
const struct process_trailer_options *opts)
{
+ size_t origlen = out->len;
size_t i;
/* If we want the whole block untouched, we can take the fast path. */
- if (!opts->only_trailers && !opts->unfold && !opts->filter) {
+ if (!opts->only_trailers && !opts->unfold && !opts->filter && !opts->separator) {
strbuf_add(out, info->trailer_start,
info->trailer_end - info->trailer_start);
return;
@@ -1150,16 +1151,26 @@ static void format_trailer_info(struct strbuf *out,
if (!opts->filter || opts->filter(&tok, opts->filter_data)) {
if (opts->unfold)
unfold_value(&val);
+
+ if (opts->separator && out->len != origlen)
+ strbuf_addbuf(out, opts->separator);
if (!opts->value_only)
strbuf_addf(out, "%s: ", tok.buf);
strbuf_addbuf(out, &val);
- strbuf_addch(out, '\n');
+ if (!opts->separator)
+ strbuf_addch(out, '\n');
}
strbuf_release(&tok);
strbuf_release(&val);
} else if (!opts->only_trailers) {
+ if (opts->separator && out->len != origlen) {
+ strbuf_addbuf(out, opts->separator);
+ }
strbuf_addstr(out, trailer);
+ if (opts->separator) {
+ strbuf_rtrim(out);
+ }
}
}
diff --git a/trailer.h b/trailer.h
index 06d417fe93..203acf4ee1 100644
--- a/trailer.h
+++ b/trailer.h
@@ -73,6 +73,7 @@ struct process_trailer_options {
int unfold;
int no_divider;
int value_only;
+ const struct strbuf *separator;
int (*filter)(const struct strbuf *, void *);
void *filter_data;
};
--
2.17.1
^ permalink raw reply related
* [PATCH v4 6/7] strbuf: separate callback for strbuf_expand:ing literals
From: Anders Waldenborg @ 2018-12-08 16:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Olga Telezhnaya, Anders Waldenborg
In-Reply-To: <20181208163647.19538-1-anders@0x63.nu>
Expanding '%n' and '%xNN' is generic functionality, so extract that from
the pretty.c formatter into a callback that can be reused.
No functional change intended
Signed-off-by: Anders Waldenborg <anders@0x63.nu>
---
pretty.c | 16 +++++-----------
strbuf.c | 21 +++++++++++++++++++++
strbuf.h | 8 ++++++++
3 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/pretty.c b/pretty.c
index c508357606..50d0b5830d 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1133,9 +1133,13 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *msg = c->message;
struct commit_list *p;
const char *arg;
- int ch;
+ size_t res;
/* these are independent of the commit */
+ res = strbuf_expand_literal_cb(sb, placeholder, NULL);
+ if (res)
+ return res;
+
switch (placeholder[0]) {
case 'C':
if (starts_with(placeholder + 1, "(auto)")) {
@@ -1154,16 +1158,6 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
*/
return ret;
}
- case 'n': /* newline */
- strbuf_addch(sb, '\n');
- return 1;
- case 'x':
- /* %x00 == NUL, %x0a == LF, etc. */
- ch = hex2chr(placeholder + 1);
- if (ch < 0)
- return 0;
- strbuf_addch(sb, ch);
- return 3;
case 'w':
if (placeholder[1] == '(') {
unsigned long width = 0, indent1 = 0, indent2 = 0;
diff --git a/strbuf.c b/strbuf.c
index f6a6cf78b9..78eecd29f7 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -380,6 +380,27 @@ void strbuf_expand(struct strbuf *sb, const char *format, expand_fn_t fn,
}
}
+size_t strbuf_expand_literal_cb(struct strbuf *sb,
+ const char *placeholder,
+ void *context)
+{
+ int ch;
+
+ switch (placeholder[0]) {
+ case 'n': /* newline */
+ strbuf_addch(sb, '\n');
+ return 1;
+ case 'x':
+ /* %x00 == NUL, %x0a == LF, etc. */
+ ch = hex2chr(placeholder + 1);
+ if (ch < 0)
+ return 0;
+ strbuf_addch(sb, ch);
+ return 3;
+ }
+ return 0;
+}
+
size_t strbuf_expand_dict_cb(struct strbuf *sb, const char *placeholder,
void *context)
{
diff --git a/strbuf.h b/strbuf.h
index fc40873b65..52e44c9ab8 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -320,6 +320,14 @@ void strbuf_expand(struct strbuf *sb,
expand_fn_t fn,
void *context);
+/**
+ * Used as callback for `strbuf_expand` to only expand literals
+ * (i.e. %n and %xNN). The context argument is ignored.
+ */
+size_t strbuf_expand_literal_cb(struct strbuf *sb,
+ const char *placeholder,
+ void *context);
+
/**
* Used as callback for `strbuf_expand()`, expects an array of
* struct strbuf_expand_dict_entry as context, i.e. pairs of
--
2.17.1
^ permalink raw reply related
* [PATCH v4 5/7] pretty: add support for "valueonly" option in %(trailers)
From: Anders Waldenborg @ 2018-12-08 16:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Olga Telezhnaya, Anders Waldenborg
In-Reply-To: <20181208163647.19538-1-anders@0x63.nu>
With the new "key=" option to %(trailers) it often makes little sense to
show the key, as it by definition already is knows which trailer is
printed there. This new "valueonly" option makes it omit the key when
printing trailers.
E.g.:
$ git show -s --pretty='%s%n%(trailers:key=Signed-off-by,valueonly)' aaaa88182
will show:
> upload-pack: fix broken if/else chain in config callback
> Jeff King <peff@peff.net>
> Junio C Hamano <gitster@pobox.com>
Signed-off-by: Anders Waldenborg <anders@0x63.nu>
---
Documentation/pretty-formats.txt | 2 ++
pretty.c | 3 ++-
t/t4205-log-pretty-formats.sh | 6 ++++++
trailer.c | 6 ++++--
trailer.h | 1 +
5 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d6add831c0..a920dd15b1 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -243,6 +243,8 @@ endif::git-rev-list[]
option was given. In same way as to for `only` it can be followed
by an equal sign and explicit value. E.g.,
`%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
+** 'valueonly[=val]': skip over the key part of the trailer line and only
+ show the value part. Also this optionally allows explicit value.
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 541a553ccc..c508357606 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1383,7 +1383,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
opts.filter_data = &filter_list;
opts.only_trailers = 1;
} else if (!match_placeholder_bool_arg(arg, "only", &arg, &opts.only_trailers) &&
- !match_placeholder_bool_arg(arg, "unfold", &arg, &opts.unfold))
+ !match_placeholder_bool_arg(arg, "unfold", &arg, &opts.unfold) &&
+ !match_placeholder_bool_arg(arg, "valueonly", &arg, &opts.value_only))
break;
}
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 54239290cf..22336c5485 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -667,6 +667,12 @@ test_expect_success 'pretty format %(trailers:key=foo,only=no) also includes non
test_cmp expect actual
'
+test_expect_success '%(trailers:key=foo,valueonly) shows only value' '
+ git log --no-walk --pretty="format:%(trailers:key=Acked-by,valueonly)" >actual &&
+ echo "A U Thor <author@example.com>" >expect &&
+ test_cmp expect actual
+'
+
test_expect_success 'trailer parsing not fooled by --- line' '
git commit --allow-empty -F - <<-\EOF &&
this is the subject
diff --git a/trailer.c b/trailer.c
index d6da555cd7..d0d9e91631 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1150,8 +1150,10 @@ static void format_trailer_info(struct strbuf *out,
if (!opts->filter || opts->filter(&tok, opts->filter_data)) {
if (opts->unfold)
unfold_value(&val);
-
- strbuf_addf(out, "%s: %s\n", tok.buf, val.buf);
+ if (!opts->value_only)
+ strbuf_addf(out, "%s: ", tok.buf);
+ strbuf_addbuf(out, &val);
+ strbuf_addch(out, '\n');
}
strbuf_release(&tok);
strbuf_release(&val);
diff --git a/trailer.h b/trailer.h
index 5255b676de..06d417fe93 100644
--- a/trailer.h
+++ b/trailer.h
@@ -72,6 +72,7 @@ struct process_trailer_options {
int only_input;
int unfold;
int no_divider;
+ int value_only;
int (*filter)(const struct strbuf *, void *);
void *filter_data;
};
--
2.17.1
^ permalink raw reply related
* [PATCH v4 3/7] pretty: single return path in %(trailers) handling
From: Anders Waldenborg @ 2018-12-08 16:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Olga Telezhnaya, Anders Waldenborg
In-Reply-To: <20181208163647.19538-1-anders@0x63.nu>
No functional change intended.
This change may not seem useful on its own, but upcoming commits will do
memory allocation in there, and a single return path makes deallocation
easier.
Signed-off-by: Anders Waldenborg <anders@0x63.nu>
---
pretty.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/pretty.c b/pretty.c
index 26efdba73a..044447e6c0 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1337,6 +1337,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
if (skip_prefix(placeholder, "(trailers", &arg)) {
struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
+ size_t ret = 0;
opts.no_divider = 1;
@@ -1350,8 +1351,9 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
}
if (*arg == ')') {
format_trailers_from_commit(sb, msg + c->subject_off, &opts);
- return arg - placeholder + 1;
+ ret = arg - placeholder + 1;
}
+ return ret;
}
return 0; /* unknown placeholder */
--
2.17.1
^ permalink raw reply related
* [PATCH v4 1/7] doc: group pretty-format.txt placeholders descriptions
From: Anders Waldenborg @ 2018-12-08 16:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Olga Telezhnaya, Anders Waldenborg
In-Reply-To: <20181208163647.19538-1-anders@0x63.nu>
The placeholders can be grouped into three kinds:
* literals
* affecting formatting of later placeholders
* expanding to information in commit
Also change the list to a definition list (using '::')
Signed-off-by: Anders Waldenborg <anders@0x63.nu>
---
Documentation/pretty-formats.txt | 235 ++++++++++++++++---------------
1 file changed, 125 insertions(+), 110 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 417b638cd8..86d804fe97 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -102,118 +102,133 @@ The title was >>t4119: test autocomputing -p<n> for traditional diff input.<<
+
The placeholders are:
-- '%H': commit hash
-- '%h': abbreviated commit hash
-- '%T': tree hash
-- '%t': abbreviated tree hash
-- '%P': parent hashes
-- '%p': abbreviated parent hashes
-- '%an': author name
-- '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1]
- or linkgit:git-blame[1])
-- '%ae': author email
-- '%aE': author email (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%ad': author date (format respects --date= option)
-- '%aD': author date, RFC2822 style
-- '%ar': author date, relative
-- '%at': author date, UNIX timestamp
-- '%ai': author date, ISO 8601-like format
-- '%aI': author date, strict ISO 8601 format
-- '%cn': committer name
-- '%cN': committer name (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%ce': committer email
-- '%cE': committer email (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%cd': committer date (format respects --date= option)
-- '%cD': committer date, RFC2822 style
-- '%cr': committer date, relative
-- '%ct': committer date, UNIX timestamp
-- '%ci': committer date, ISO 8601-like format
-- '%cI': committer date, strict ISO 8601 format
-- '%d': ref names, like the --decorate option of linkgit:git-log[1]
-- '%D': ref names without the " (", ")" wrapping.
-- '%e': encoding
-- '%s': subject
-- '%f': sanitized subject line, suitable for a filename
-- '%b': body
-- '%B': raw body (unwrapped subject and body)
+- Placeholders that expand to a single literal character:
+'%n':: newline
+'%%':: a raw '%'
+'%x00':: print a byte from a hex code
+
+- Placeholders that affect formatting of later placeholders:
+'%Cred':: switch color to red
+'%Cgreen':: switch color to green
+'%Cblue':: switch color to blue
+'%Creset':: reset color
+'%C(...)':: color specification, as described under Values in the
+ "CONFIGURATION FILE" section of linkgit:git-config[1]. By
+ default, colors are shown only when enabled for log output
+ (by `color.diff`, `color.ui`, or `--color`, and respecting
+ the `auto` settings of the former if we are going to a
+ terminal). `%C(auto,...)` is accepted as a historical
+ synonym for the default (e.g., `%C(auto,red)`). Specifying
+ `%C(always,...) will show the colors even when color is
+ not otherwise enabled (though consider just using
+ `--color=always` to enable color for the whole output,
+ including this format and anything else git might color).
+ `auto` alone (i.e. `%C(auto)`) will turn on auto coloring
+ on the next placeholders until the color is switched
+ again.
+'%m':: left (`<`), right (`>`) or boundary (`-`) mark
+'%w([<w>[,<i1>[,<i2>]]])':: switch line wrapping, like the -w option of
+ linkgit:git-shortlog[1].
+'%<(<N>[,trunc|ltrunc|mtrunc])':: make the next placeholder take at
+ least N columns, padding spaces on
+ the right if necessary. Optionally
+ truncate at the beginning (ltrunc),
+ the middle (mtrunc) or the end
+ (trunc) if the output is longer than
+ N columns. Note that truncating
+ only works correctly with N >= 2.
+'%<|(<N>)':: make the next placeholder take at least until Nth
+ columns, padding spaces on the right if necessary
+'%>(<N>)', '%>|(<N>)':: similar to '%<(<N>)', '%<|(<N>)' respectively,
+ but padding spaces on the left
+'%>>(<N>)', '%>>|(<N>)':: similar to '%>(<N>)', '%>|(<N>)'
+ respectively, except that if the next
+ placeholder takes more spaces 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)
+
+- Placeholders that expand to information extracted from the commit:
+'%H':: commit hash
+'%h':: abbreviated commit hash
+'%T':: tree hash
+'%t':: abbreviated tree hash
+'%P':: parent hashes
+'%p':: abbreviated parent hashes
+'%an':: author name
+'%aN':: author name (respecting .mailmap, see linkgit:git-shortlog[1]
+ or linkgit:git-blame[1])
+'%ae':: author email
+'%aE':: author email (respecting .mailmap, see linkgit:git-shortlog[1]
+ or linkgit:git-blame[1])
+'%ad':: author date (format respects --date= option)
+'%aD':: author date, RFC2822 style
+'%ar':: author date, relative
+'%at':: author date, UNIX timestamp
+'%ai':: author date, ISO 8601-like format
+'%aI':: author date, strict ISO 8601 format
+'%cn':: committer name
+'%cN':: committer name (respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
+'%ce':: committer email
+'%cE':: committer email (respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
+'%cd':: committer date (format respects --date= option)
+'%cD':: committer date, RFC2822 style
+'%cr':: committer date, relative
+'%ct':: committer date, UNIX timestamp
+'%ci':: committer date, ISO 8601-like format
+'%cI':: committer date, strict ISO 8601 format
+'%d':: ref names, like the --decorate option of linkgit:git-log[1]
+'%D':: ref names without the " (", ")" wrapping.
+'%e':: encoding
+'%s':: subject
+'%f':: sanitized subject line, suitable for a filename
+'%b':: body
+'%B':: raw body (unwrapped subject and body)
ifndef::git-rev-list[]
-- '%N': commit notes
+'%N':: commit notes
endif::git-rev-list[]
-- '%GG': raw verification message from GPG for a signed commit
-- '%G?': show "G" for a good (valid) signature,
- "B" for a bad signature,
- "U" for a good signature with unknown validity,
- "X" for a good signature that has expired,
- "Y" for a good signature made by an expired key,
- "R" for a good signature made by a revoked key,
- "E" if the signature cannot be checked (e.g. missing key)
- and "N" for no signature
-- '%GS': show the name of the signer for a signed commit
-- '%GK': show the key used to sign a signed commit
-- '%GF': show the fingerprint of the key used to sign a signed commit
-- '%GP': show the fingerprint of the primary key whose subkey was used
- to sign a signed commit
-- '%gD': reflog selector, e.g., `refs/stash@{1}` or
- `refs/stash@{2 minutes ago`}; the format follows the rules described
- for the `-g` option. The portion before the `@` is the refname as
- given on the command line (so `git log -g refs/heads/master` would
- yield `refs/heads/master@{0}`).
-- '%gd': shortened reflog selector; same as `%gD`, but the refname
- portion is shortened for human readability (so `refs/heads/master`
- becomes just `master`).
-- '%gn': reflog identity name
-- '%gN': reflog identity name (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%ge': reflog identity email
-- '%gE': reflog identity email (respecting .mailmap, see
- linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- '%gs': reflog subject
-- '%Cred': switch color to red
-- '%Cgreen': switch color to green
-- '%Cblue': switch color to blue
-- '%Creset': reset color
-- '%C(...)': color specification, as described under Values in the
- "CONFIGURATION FILE" section of linkgit:git-config[1].
- By default, colors are shown only when enabled for log output (by
- `color.diff`, `color.ui`, or `--color`, and respecting the `auto`
- settings of the former if we are going to a terminal). `%C(auto,...)`
- is accepted as a historical synonym for the default (e.g.,
- `%C(auto,red)`). Specifying `%C(always,...) will show the colors
- even when color is not otherwise enabled (though consider
- just using `--color=always` to enable color for the whole output,
- including this format and anything else git might color). `auto`
- alone (i.e. `%C(auto)`) will turn on auto coloring on the next
- placeholders until the color is switched again.
-- '%m': left (`<`), right (`>`) or boundary (`-`) mark
-- '%n': newline
-- '%%': a raw '%'
-- '%x00': print a byte from a hex code
-- '%w([<w>[,<i1>[,<i2>]]])': switch line wrapping, like the -w option of
- linkgit:git-shortlog[1].
-- '%<(<N>[,trunc|ltrunc|mtrunc])': make the next placeholder take at
- least N columns, padding spaces on the right if necessary.
- Optionally truncate at the beginning (ltrunc), the middle (mtrunc)
- or the end (trunc) if the output is longer than N columns.
- Note that truncating only works correctly with N >= 2.
-- '%<|(<N>)': make the next placeholder take at least until Nth
- columns, padding spaces on the right if necessary
-- '%>(<N>)', '%>|(<N>)': similar to '%<(<N>)', '%<|(<N>)'
- respectively, but padding spaces on the left
-- '%>>(<N>)', '%>>|(<N>)': similar to '%>(<N>)', '%>|(<N>)'
- respectively, except that if the next placeholder takes more spaces
- 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[:options]): display the trailers of the body as interpreted
- by linkgit:git-interpret-trailers[1]. The `trailers` string may be
- followed by a colon and zero or more comma-separated options. If the
- `only` option is given, omit non-trailer lines from the trailer block.
- If the `unfold` option is given, behave as if interpret-trailer's
- `--unfold` option was given. E.g., `%(trailers:only,unfold)` to do
- both.
+'%GG':: raw verification message from GPG for a signed commit
+'%G?':: show "G" for a good (valid) signature,
+ "B" for a bad signature,
+ "U" for a good signature with unknown validity,
+ "X" for a good signature that has expired,
+ "Y" for a good signature made by an expired key,
+ "R" for a good signature made by a revoked key,
+ "E" if the signature cannot be checked (e.g. missing key)
+ and "N" for no signature
+'%GS':: show the name of the signer for a signed commit
+'%GK':: show the key used to sign a signed commit
+'%GF':: show the fingerprint of the key used to sign a signed commit
+'%GP':: show the fingerprint of the primary key whose subkey was used
+ to sign a signed commit
+'%gD':: reflog selector, e.g., `refs/stash@{1}` or `refs/stash@{2
+ minutes ago`}; the format follows the rules described for the
+ `-g` option. The portion before the `@` is the refname as
+ given on the command line (so `git log -g refs/heads/master`
+ would yield `refs/heads/master@{0}`).
+'%gd':: shortened reflog selector; same as `%gD`, but the refname
+ portion is shortened for human readability (so
+ `refs/heads/master` becomes just `master`).
+'%gn':: reflog identity name
+'%gN':: reflog identity name (respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
+'%ge':: reflog identity email
+'%gE':: reflog identity email (respecting .mailmap, see
+ linkgit:git-shortlog[1] or linkgit:git-blame[1])
+'%gs':: reflog subject
+'%(trailers[:options])':: display the trailers of the body as
+ interpreted by
+ linkgit:git-interpret-trailers[1]. The
+ `trailers` string may be followed by a colon
+ and zero or more comma-separated options:
+** 'only': omit non-trailer lines from the trailer block.
+** 'unfold': make it behave as if interpret-trailer's `--unfold`
+ option was given. E.g., `%(trailers:only,unfold)` unfolds and
+ shows all trailer lines.
NOTE: Some placeholders may depend on other options given to the
revision traversal engine. For example, the `%g*` reflog options will
--
2.17.1
^ permalink raw reply related
* [PATCH v4 2/7] pretty: allow %(trailers) options with explicit value
From: Anders Waldenborg @ 2018-12-08 16:36 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Olga Telezhnaya, Anders Waldenborg
In-Reply-To: <20181208163647.19538-1-anders@0x63.nu>
In addition to old %(trailers:only) it is now allowed to write
%(trailers:only=yes)
By itself this only gives (the not quite so useful) possibility to have
users change their mind in the middle of a formatting
string (%(trailers:only=true,only=false)). However, it gives users the
opportunity to override defaults from future options.
Signed-off-by: Anders Waldenborg <anders@0x63.nu>
---
Documentation/pretty-formats.txt | 14 ++++++++++----
pretty.c | 32 +++++++++++++++++++++++++++-----
t/t4205-log-pretty-formats.sh | 18 ++++++++++++++++++
3 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 86d804fe97..d33b072eb2 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -225,10 +225,16 @@ endif::git-rev-list[]
linkgit:git-interpret-trailers[1]. The
`trailers` string may be followed by a colon
and zero or more comma-separated options:
-** 'only': omit non-trailer lines from the trailer block.
-** 'unfold': make it behave as if interpret-trailer's `--unfold`
- option was given. E.g., `%(trailers:only,unfold)` unfolds and
- shows all trailer lines.
+** 'only[=val]': select whether non-trailer lines from the trailer
+ block should be included. The `only` keyword may optionally be
+ followed by an equal sign and one of `true`, `on`, `yes` to omit or
+ `false`, `off`, `no` to show the non-trailer lines. If option is
+ given without value it is enabled. If given multiple times the last
+ value is used.
+** 'unfold[=val]': make it behave as if interpret-trailer's `--unfold`
+ option was given. In same way as to for `only` it can be followed
+ by an equal sign and explicit value. E.g.,
+ `%(trailers:only,unfold=true)` unfolds and shows all trailer lines.
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 b83a3ecd23..26efdba73a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1074,6 +1074,31 @@ static int match_placeholder_arg(const char *to_parse, const char *candidate,
return 0;
}
+static int match_placeholder_bool_arg(const char *to_parse, const char *candidate,
+ const char **end, int *val)
+{
+ const char *p;
+ if (!skip_prefix(to_parse, candidate, &p))
+ return 0;
+
+ if (match_placeholder_arg(p, "=no", end) ||
+ match_placeholder_arg(p, "=off", end) ||
+ match_placeholder_arg(p, "=false", end)) {
+ *val = 0;
+ return 1;
+ }
+
+ if (match_placeholder_arg(p, "", end) ||
+ match_placeholder_arg(p, "=yes", end) ||
+ match_placeholder_arg(p, "=on", end) ||
+ match_placeholder_arg(p, "=true", end)) {
+ *val = 1;
+ return 1;
+ }
+
+ return 0;
+}
+
static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1318,11 +1343,8 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
if (*arg == ':') {
arg++;
for (;;) {
- if (match_placeholder_arg(arg, "only", &arg))
- opts.only_trailers = 1;
- else if (match_placeholder_arg(arg, "unfold", &arg))
- opts.unfold = 1;
- else
+ if (!match_placeholder_bool_arg(arg, "only", &arg, &opts.only_trailers) &&
+ !match_placeholder_bool_arg(arg, "unfold", &arg, &opts.unfold))
break;
}
}
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 978a8a66ff..63730a4ec0 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -578,6 +578,24 @@ test_expect_success '%(trailers:only) shows only "key: value" trailers' '
test_cmp expect actual
'
+test_expect_success '%(trailers:only=yes) shows only "key: value" trailers' '
+ git log --no-walk --pretty=format:"%(trailers:only=yes)" >actual &&
+ grep -v patch.description <trailers >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailers:only=no) shows all trailers' '
+ git log --no-walk --pretty=format:"%(trailers:only=no)" >actual &&
+ cat trailers >expect &&
+ test_cmp expect actual
+'
+
+test_expect_success '%(trailers:only=no,only=true) shows only "key: value" trailers' '
+ git log --no-walk --pretty=format:"%(trailers:only=yes)" >actual &&
+ grep -v patch.description <trailers >expect &&
+ test_cmp expect actual
+'
+
test_expect_success '%(trailers:unfold) unfolds trailers' '
git log --no-walk --pretty="%(trailers:unfold)" >actual &&
{
--
2.17.1
^ permalink raw reply related
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