line-log: integrate -L with the standard log output pipeline
cover line-log: integrate -L with the standard log output pipeline
Since its introduction, git log -L has short-circuited from log_tree_commit() into its own output function, bypassing log_tree_diff() and log_tree_diff_flush(). This skips no_free save/restore, always_show_header, diff_free() cleanup, and means that pickaxe (-S, -G, --find-object) and --diff-filter cannot suppress commits whose pairs are all filtered out, because show_log() runs before diffcore_std().
This series restructures the flow so that -L goes through the same log_tree_diff() -> log_tree_diff_flush() path as normal single-parent and merge diffs, then uses that to enable several non-patch diff formats.
Patch 1: revision: move -L setup before output_format-to-diff derivation
Preparatory reorder in setup_revisions(). The -L block sets a default DIFF_FORMAT_PATCH when no format is requested; move it before the derivation of revs->diff from output_format so the default is visible to that check. No behavior change on its own.
Patch 2: line-log: integrate -L output with the standard log-tree pipeline
Rename line_log_print() to line_log_queue_pairs(), stripping it down to only queue pre-computed filepairs. log_tree_diff_flush() handles show_log(), diffcore_std(), and diff_flush(). This fixes pickaxe and --diff-filter suppression, and aligns the commit/diff separator with the rest of log output. Also rejects --full-diff, which is meaningless when filepairs are pre-computed.
Patch 3: line-log: allow non-patch diff formats with -L
Expand the allowlist to accept --raw, --name-only, --name-status, and --summary. These only read filepair metadata already set by the line-log machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call compute_diffstat() on full blob content and would show whole-file statistics rather than range-scoped ones.
Michael Montalbo (3):
revision: move -L setup before output_format-to-diff derivation
line-log: integrate -L output with the standard log-tree pipeline
line-log: allow non-patch diff formats with -L
Documentation/line-range-options.adoc | 10 +- line-log.c | 30 ++---- line-log.h | 2 +- log-tree.c | 9 +- revision.c | 25 +++-- t/t4211-line-log.sh | 99 ++++++++++++++++--- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 8 files changed, 120 insertions(+), 57 deletions(-)
base-commit: 9f223ef1c026d91c7ac68cc0211bde255dda6199
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2094
Signature · 1 line
gitgitgadget
PATCH 1/3 revision: move -L setup before output_format-to-diff derivation
From: Michael Montalbo <[email protected]>
The line_level_traverse block sets a default DIFF_FORMAT_PATCH when no output format has been explicitly requested. This default must be visible to the "Did the user ask for any diff output?" check that derives revs->diff from revs->diffopt.output_format.
Currently the -L block runs after that derivation, so revs->diff stays 0 when no explicit format is given. This does not matter yet because log_tree_commit() short-circuits into line_log_print() before consulting revs->diff, but the next commit will route -L through the normal log_tree_diff() path, which checks revs->diff.
Move the block above the derivation so the default DIFF_FORMAT_PATCH is in place when revs->diff is computed. No behavior change on its own.
--- revision.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
revision.c +8 -8
@@ -3112,6 +3112,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sobject_context_release (&oc); } +if (revs->line_level_traverse) { +if (want_ancestry (revs)) + revs->limited =1 ; + revs->topo_order =1 ; +if (!revs->diffopt.output_format) + revs->diffopt.output_format =DIFF_FORMAT_PATCH ; + } +/* Did the user ask for any diff output? Run the diff! */ if (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT ) revs->diff =1 ;
@@ -3125,14 +3133,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sif (revs->diffopt.objfind) revs->simplify_history =0 ; -if (revs->line_level_traverse) { -if (want_ancestry (revs)) - revs->limited =1 ; - revs->topo_order =1 ; -if (!revs->diffopt.output_format) - revs->diffopt.output_format =DIFF_FORMAT_PATCH ; - } -if (revs->topo_order && !generation_numbers_enabled (the_repository)) revs->limited =1 ;
Signature · 1 line
gitgitgadget
PATCH 2/3 line-log: integrate -L output with the standard log-tree pipeline
In reply to Michael Montalbo via GitGitGadgetFrom: Michael Montalbo <[email protected]>
git log -L has bypassed log_tree_diff() and log_tree_diff_flush() since the feature was introduced, short-circuiting from log_tree_commit() directly into line_log_print(). This skips the no_free save/restore (noted in a NEEDSWORK comment added by f8781bfda3), the always_show_header fallback, show_diff_of_diff(), and diff_free() cleanup.
Restructure so that -L flows through log_tree_diff() -> log_tree_diff_flush(), the same path used by the normal single-parent and merge diff codepaths:
- Rename line_log_print() to line_log_queue_pairs() and strip it down to just queuing pre-computed filepairs. The show_log(), separator, diffcore_std(), and diff_flush() calls are removed since log_tree_diff_flush() handles all of those.
- In log_tree_diff(), call line_log_queue_pairs() then log_tree_diff_flush(), mirroring the diff_tree_oid() + flush pattern used by the single-parent and merge codepaths.
- Remove the early return in log_tree_commit() that bypassed no_free save/restore, always_show_header, and diff_free().
Because show_log() is now deferred until after diffcore_std() inside log_tree_diff_flush(), pickaxe (-S, -G, --find-object) and --diff-filter now properly suppress commits when all pairs are filtered out.
The blank-line separator between commit header and diff changes
slightly: the old code printed one unconditionally, while log_tree_diff_flush() only emits one for verbose headers. This matches the rest of log output.
Also reject --full-diff, which is meaningless with -L: the filepairs are pre-computed during the history walk and scoped to tracked paths, so there is no tree diff to widen.
Update tests accordingly.
--- line-log.c | 30 ++++------- line-log.h | 2 +- log-tree.c | 9 ++-- revision.c | 2 + t/t4211-line-log.sh | 52 ++++++++++++++----- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 7 files changed, 56 insertions(+), 41 deletions(-)
line-log.c +9 -21
@@ -13,7 +13,6 @@#include "revision.h" #include "xdiff-interface.h" #include "strbuf.h" -#include "log-tree.h" #include "line-log.h" #include "setup.h" #include "strvec.h"
@@ -1004,29 +1003,18 @@ static int process_all_files(struct line_log_data **range_out,return changed; } -int line_log_print (struct rev_info *rev,struct commit *commit) +void line_log_queue_pairs (struct rev_info *rev,struct commit *commit) { -show_log (rev); -if (!(rev->diffopt.output_format &DIFF_FORMAT_NO_OUTPUT )) { -struct line_log_data *range =lookup_line_range (rev, commit); -struct line_log_data *r; -const char *prefix =diff_line_prefix (&rev->diffopt); - -fprintf (rev->diffopt.file,"%s\n" , prefix); - -for (r = range; r; r = r->next) { -if (r->pair) { -struct diff_filepair *p = -diff_filepair_dup (r->pair); - p->line_ranges = &r->ranges; -diff_q (&diff_queued_diff, p); - } - } +struct line_log_data *range =lookup_line_range (rev, commit); +struct line_log_data *r; -diffcore_std (&rev->diffopt); -diff_flush (&rev->diffopt); +for (r = range; r; r = r->next) { +if (r->pair) { +struct diff_filepair *p =diff_filepair_dup (r->pair); + p->line_ranges = &r->ranges; +diff_q (&diff_queued_diff, p); + } } -return 1 ; } static int bloom_filter_check(struct rev_info *rev,
line-log.h +1 -1
@@ -46,7 +46,7 @@ int line_log_filter(struct rev_info *rev);int line_log_process_ranges_arbitrary_commit (struct rev_info *rev,struct commit *commit); -int line_log_print (struct rev_info *rev,struct commit *commit); +void line_log_queue_pairs (struct rev_info *rev,struct commit *commit);void line_log_free (struct rev_info *rev);
log-tree.c +5 -4
@@ -1105,6 +1105,11 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct logif (!all_need_diff && !opt->merges_need_diff)return 0 ; +if (opt->line_level_traverse) { +line_log_queue_pairs (opt, commit); +return log_tree_diff_flush (opt); + } +parse_commit_or_die (commit); oid =get_commit_tree_oid (commit);
@@ -1179,10 +1184,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit) opt->loginfo = &log; opt->diffopt.no_free =1 ; -/* NEEDSWORK: no restoring of no_free? Why? */ -if (opt->line_level_traverse) -return line_log_print (opt, commit); -if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)fprintf (opt->diffopt.file,"\n%s\n" , opt->break_bar); shown =log_tree_diff (opt, commit, &log);
revision.c +2 -0
@@ -3181,6 +3181,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sif (revs->line_level_traverse && (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT )))die (_ ("-L does not yet support diff formats besides -p and -s" )); +if (revs->line_level_traverse && revs->full_diff) +die (_ ("-L is not compatible with --full-diff" ));if (revs->expand_tabs_in_log <0 ) revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
t/t4211-line-log.sh +39 -13
94 lines not on this page · raw message
t/t4211/sha1/expect.parallel-change-f-to-main +0 -1
@@ -5,7 +5,6 @@ Date: Fri Apr 12 16:16:24 2013 +0200 Merge across the rename - commit 6ce3c4ff690136099bb17e1a8766b75764726ea7 Author: Thomas Rast <[email protected]> Date: Thu Feb 28 10:49:50 2013 +0100
t/t4211/sha256/expect.parallel-change-f-to-main +0 -1
@@ -5,7 +5,6 @@ Date: Fri Apr 12 16:16:24 2013 +0200 Merge across the rename - commit 4f7a58195a92c400e28a2354328587f1ff14fb77f5cf894536f17ccbc72931b9 Author: Thomas Rast <[email protected]> Date: Thu Feb 28 10:49:50 2013 +0100
Signature · 1 line
gitgitgadget
PATCH 3/3 line-log: allow non-patch diff formats with -L
In reply to Michael Montalbo via GitGitGadgetFrom: Michael Montalbo <[email protected]>
Now that -L flows through log_tree_diff_flush() and diff_flush(), metadata-only diff formats work because they only read filepair fields (status, mode, path, oid) already set on the pre-computed pairs.
Expand the allowlist in setup_revisions() to also accept --raw, --name-only, --name-status, and --summary. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call compute_diffstat() on full blob content and would show whole-file statistics rather than range-scoped ones.
--- Documentation/line-range-options.adoc | 10 +++--- revision.c | 7 ++-- t/t4211-line-log.sh | 47 +++++++++++++++++++++++++-- 3 files changed, 56 insertions(+), 8 deletions(-)
Documentation/line-range-options.adoc +6 -4
@@ -8,12 +8,14 @@ give zero or one positive revision arguments, and _<start>_ and _<end>_ (or _<funcname>_) must exist in the starting revision. You can specify this option more than once. Implies `--patch`. - Patch output can be suppressed using `--no-patch`, but other diff formats - (namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`, - `--name-only`, `--name-status`, `--check`) are not currently implemented. + Patch output can be suppressed using `--no-patch`. + Non-patch diff formats `--raw`, `--name-only`, `--name-status`, + and `--summary` are supported. Diff stat formats + (`--stat`, `--numstat`, `--shortstat`, `--dirstat`) are not + currently implemented. + Patch formatting options such as `--word-diff`, `--color-moved`, `--no-prefix`, and whitespace options (`-w`, `-b`) are supported, -as are pickaxe options (`-S`, `-G`). +as are pickaxe options (`-S`, `-G`) and `--diff-filter`. + include::line-range-format.adoc[]
revision.c +5 -2
@@ -3179,8 +3179,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sdie (_ ("the option '%s' requires '%s'" ),"--grep-reflog" ,"--walk-reflogs" );if (revs->line_level_traverse && - (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT ))) -die (_ ("-L does not yet support diff formats besides -p and -s" )); + (revs->diffopt.output_format & + ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT | +DIFF_FORMAT_RAW |DIFF_FORMAT_NAME | +DIFF_FORMAT_NAME_STATUS |DIFF_FORMAT_SUMMARY ))) +die (_ ("-L does not yet support the requested diff format" ));if (revs->line_level_traverse && revs->full_diff)die (_ ("-L is not compatible with --full-diff" ));
t/t4211-line-log.sh +45 -2
57 lines not on this page · raw message
Signature · 1 line
gitgitgadget
Quoted 10 lines · Michael Montalbo via GitGitGadget
Since its introduction, git log -L has short-circuited from
log_tree_commit()into its own output function, bypassinglog_tree_diff()andlog_tree_diff_flush(). This skipsno_freesave/restore,always_show_header,diff_free()cleanup, and means that pickaxe (-S, -G, --find-object) and --diff-filter cannot suppress commits whose pairs are all filtered out, becauseshow_log()runs beforediffcore_std().This series restructures the flow so that -L goes through the same
log_tree_diff()->log_tree_diff_flush()path as normal single-parent and merge diffs, then uses that to enable several non-patch diff formats.
This unfortunately saw no reviews and
https://lore.kernel.org/git/[email protected]/
does not show the previous rouns so I am assuming nobody is interested in the topic?
Or are people more busily writing their own patches than reviewing others' patches? Unfortunately that is not sustainable.
Quoted 44 lines
Patch 1: revision: move -L setup before output_format-to-diff derivation
Preparatory reorder in
setup_revisions(). The -L block sets a defaultDIFF_FORMAT_PATCHwhen no format is requested; move it before the derivation ofrevs->difffromoutput_formatso the default is visible to that check. No behavior change on its own.Patch 2: line-log: integrate -L output with the standard log-tree pipeline
Rename
line_log_print()toline_log_queue_pairs(), stripping it down to only queue pre-computed filepairs.log_tree_diff_flush()handlesshow_log(),diffcore_std(), anddiff_flush(). This fixes pickaxe and --diff-filter suppression, and aligns the commit/diff separator with the rest of log output. Also rejects --full-diff, which is meaningless when filepairs are pre-computed.Patch 3: line-log: allow non-patch diff formats with -L
Expand the allowlist to accept --raw, --name-only, --name-status, and --summary. These only read filepair metadata already set by the line-log machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call
compute_diffstat()on full blob content and would show whole-file statistics rather than range-scoped ones.Michael Montalbo (3):
revision: move -L setup before output_format-to-diff derivation
line-log: integrate -L output with the standard log-tree pipeline
line-log: allow non-patch diff formats with -LDocumentation/line-range-options.adoc | 10 +- line-log.c | 30 ++---- line-log.h | 2 +- log-tree.c | 9 +- revision.c | 25 +++-- t/t4211-line-log.sh | 99 ++++++++++++++++--- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 8 files changed, 120 insertions(+), 57 deletions(-)base-commit:
9f223ef1c026d91c7ac68cc0211bde255dda6199
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2094
Hi Michael,
Quoted 11 lines · Michael Montalbo via GitGitGadget
Since its introduction, git log -L has short-circuited from
log_tree_commit()into its own output function, bypassinglog_tree_diff()andlog_tree_diff_flush(). This skipsno_freesave/restore,always_show_header,diff_free()cleanup, and means that pickaxe (-S, -G, --find-object) and --diff-filter cannot suppress commits whose pairs are all filtered out, becauseshow_log()runs beforediffcore_std().This series restructures the flow so that -L goes through the same
log_tree_diff()->log_tree_diff_flush()path as normal single-parent and merge diffs, then uses that to enable several non-patch diff formats.
Cleanup by itself to shrink the number of concepts in the code is already a good thing IMO, so getting additional features out of it is even nicer.
Quoted 6 lines
Patch 1: revision: move -L setup before output_format-to-diff derivation
Preparatory reorder in
setup_revisions(). The -L block sets a defaultDIFF_FORMAT_PATCHwhen no format is requested; move it before the derivation ofrevs->difffromoutput_formatso the default is visible to that check. No behavior change on its own.
Straightforward, nice.
Quoted 9 lines
Patch 2: line-log: integrate -L output with the standard log-tree pipeline
Rename
line_log_print()toline_log_queue_pairs(), stripping it down to only queue pre-computed filepairs.log_tree_diff_flush()handlesshow_log(),diffcore_std(), anddiff_flush(). This fixes pickaxe and --diff-filter suppression, and aligns the commit/diff separator with the rest of log output. Also rejects --full-diff, which is meaningless when filepairs are pre-computed.
At first I questioned the removal of the DIFF_FORMAT_NO_OUTPUT conditional in line_log_queue_pairs, but now that it only queues pairs it shouldn't be checking output formats. Good.
I also noted that log_tree_diff() returns the result of log_tree_diff_flush() in the -L case, which is a bit different from the other patterns. I think the difference is that the other cases have some conditional logic around the log_tree_diff_flush cases (?) but I'm not sure. Perhaps that branch should also be looking at opt->loginfo ?
Finally, I wonder if in describing the removal of the early return:
Quoted 2 lines
- Remove the early return in
log_tree_commit()that bypassedno_freesave/restore,always_show_header, anddiff_free().
we might want to be more explicit that this is _because_ line-level diff is now handled in the regular pipeline?
[I suppose we could, in theory, split the rejection of --full-diff to a separate prep commit, idk.j]
Quoted 7 lines
Patch 3: line-log: allow non-patch diff formats with -L
Expand the allowlist to accept --raw, --name-only, --name-status, and --summary. These only read filepair metadata already set by the line-log machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call
compute_diffstat()on full blob content and would show whole-file statistics rather than range-scoped ones.
Short and sweet.
The stat formats are kind of like --full-diff, and I think they should probably all be rejected or all allowed: since the stats are based on the full-diff, it makes sense to enable them if we can also make -L + --full-diff semantically sensible.
Otherwise, we'd need to find a way to make the stat formats scoped for -L.
Quoted 16 lines
Michael Montalbo (3):
revision: move -L setup before output_format-to-diff derivation
line-log: integrate -L output with the standard log-tree pipeline
line-log: allow non-patch diff formats with -LDocumentation/line-range-options.adoc | 10 +- line-log.c | 30 ++---- line-log.h | 2 +- log-tree.c | 9 +- revision.c | 25 +++-- t/t4211-line-log.sh | 99 ++++++++++++++++--- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 8 files changed, 120 insertions(+), 57 deletions(-)
Quoted 6 lines
base-commit:
9f223ef1c026d91c7ac68cc0211bde255dda6199
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2094
--
gitgitgadget
A few other comments:
- Tests should use test_grep; some do, but some don't.
- There is one occurrence of "sed | grep" that I wonder if we want to rewrite to avoid issues with exit status one side of the pipe?
Thanks for working on this!
[Apologies for the unusual review format; this was easier for me at the moment than digging up the individual patches, and I don't think _most_ of the review would benefit from spreading out across multiple mails.]
Signature · 1 line
D. Ben Knoble
A message that is not in the archive (sent off-list or not yet received): CALnO6CBh7nDCwT=u1xSN2c6_x88t_gNfAaT_B4PzYKr=5i_bNA@mail.gmail.com
Quoted 3 lines · D. Ben Knoble
Hi Michael,
Hi Ben,
Thanks for the thorough review!
Quoted 37 lines · Michael Montalbo via GitGitGadget
Quoted 11 lines
Since its introduction, git log -L has short-circuited from
log_tree_commit()into its own output function, bypassinglog_tree_diff()andlog_tree_diff_flush(). This skipsno_freesave/restore,always_show_header,diff_free()cleanup, and means that pickaxe (-S, -G, --find-object) and --diff-filter cannot suppress commits whose pairs are all filtered out, becauseshow_log()runs beforediffcore_std().This series restructures the flow so that -L goes through the same
log_tree_diff()->log_tree_diff_flush()path as normal single-parent and merge diffs, then uses that to enable several non-patch diff formats.Cleanup by itself to shrink the number of concepts in the code is already a good thing IMO, so getting additional features out of it is even nicer.
Quoted 6 lines
Patch 1: revision: move -L setup before output_format-to-diff derivation
Preparatory reorder in
setup_revisions(). The -L block sets a defaultDIFF_FORMAT_PATCHwhen no format is requested; move it before the derivation ofrevs->difffromoutput_formatso the default is visible to that check. No behavior change on its own.Straightforward, nice.
Quoted 9 lines
Patch 2: line-log: integrate -L output with the standard log-tree pipeline
Rename
line_log_print()toline_log_queue_pairs(), stripping it down to only queue pre-computed filepairs.log_tree_diff_flush()handlesshow_log(),diffcore_std(), anddiff_flush(). This fixes pickaxe and --diff-filter suppression, and aligns the commit/diff separator with the rest of log output. Also rejects --full-diff, which is meaningless when filepairs are pre-computed.
Quoted 4 lines
At first I questioned the removal of the
DIFF_FORMAT_NO_OUTPUTconditional inline_log_queue_pairs, but now that it only queues pairs it shouldn't be checking output formats. Good.I also noted that
log_tree_diff()returns the result oflog_tree_diff_flush()in the -L case, which is a bit different from the other patterns. I think the difference is that the other cases have some conditional logic around thelog_tree_diff_flushcases (?) but I'm not sure. Perhaps that branch should also be looking atopt->loginfo?
Good catch. In practice I think they agree, since log_tree_diff_flush() returns 1 exactly when it calls show_log() which consumes loginfo, but matching the existing convention is cleaner. Will update.
Quoted 7 lines
Finally, I wonder if in describing the removal of the early return:
Quoted 2 lines
- Remove the early return in
log_tree_commit()that bypassedno_freesave/restore,always_show_header, anddiff_free().we might want to be more explicit that this is _because_ line-level diff is now handled in the regular pipeline?
Agreed, will reword to: "Remove the early return in log_tree_commit() that is no longer needed now that -L output flows through log_tree_diff() and log_tree_diff_flush(); this restores no_free save/restore, always_show_header, and diff_free() cleanup."
Quoted 2 lines
[I suppose we could, in theory, split the rejection of --full-diff to a separate prep commit, idk.j]
It felt natural to me to put alongside the integration since --full-diff is not yet implementable with pre-computed filepairs. Happy to split it out if you feel strongly though.
Quoted 9 lines
Quoted 7 lines
Patch 3: line-log: allow non-patch diff formats with -L
Expand the allowlist to accept --raw, --name-only, --name-status, and --summary. These only read filepair metadata already set by the line-log machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call
compute_diffstat()on full blob content and would show whole-file statistics rather than range-scoped ones.
Quoted 6 lines
Short and sweet.
The stat formats are kind of like --full-diff, and I think they should probably all be rejected or all allowed: since the stats are based on the full-diff, it makes sense to enable them if we can also make -L + --full-diff semantically sensible.
Otherwise, we'd need to find a way to make the stat formats scoped for -L.
I am working on a follow up series that takes the second path you suggest: it adds a line-range filter in diffcore_std() that clips insertions and deletions to the tracked ranges before compute_diffstat() runs, so --stat, --numstat, etc. report range-scoped numbers. That series builds on top of these three patches, which is why stats remain blocked here.
For --full-diff, thinking about it more, the semantics would actually be well-defined: "filter commits by line range, but show the full diff for those commits." Right now, there might be a higher implementation barrier, though. The line-log machinery fuses commit filtering with diff generation, so there is no separate "full diff" to fall back to for display. I will soften the rejection to "not yet supported" rather than "incompatible," since it could be wired up if someone separates the two concerns.
Quoted 26 lines
Quoted 18 lines
Michael Montalbo (3):
revision: move -L setup before output_format-to-diff derivation
line-log: integrate -L output with the standard log-tree pipeline
line-log: allow non-patch diff formats with -LDocumentation/line-range-options.adoc | 10 +- line-log.c | 30 ++---- line-log.h | 2 +- log-tree.c | 9 +- revision.c | 25 +++-- t/t4211-line-log.sh | 99 ++++++++++++++++--- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 8 files changed, 120 insertions(+), 57 deletions(-)Quoted 6 lines
base-commit:
9f223ef1c026d91c7ac68cc0211bde255dda6199
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2094
--
gitgitgadget
Quoted 5 lines
A few other comments:
- Tests should use
test_grep; some do, but some don't.
- There is one occurrence of "sed | grep" that I wonder if we want to rewrite to avoid issues with exit status one side of the pipe?
Will fix both these issues.
Quoted 4 lines
Thanks for working on this!
[Apologies for the unusual review format; this was easier for me at the moment than digging up the individual patches, and I don't think _most_ of the review would benefit from spreading out across multiple mails.]
Thank you too! This review format worked fine for me :)
Quoted 2 lines
--
D. Ben Knoble
v2 cover line-log: integrate -L with the standard log output pipeline
In reply to Michael Montalbo via GitGitGadgetSince its introduction, git log -L has short-circuited from log_tree_commit() into its own output function, bypassing log_tree_diff() and log_tree_diff_flush(). This skips no_free save/restore, always_show_header, diff_free() cleanup, and means that pickaxe (-S, -G, --find-object) and --diff-filter cannot suppress commits whose pairs are all filtered out, because show_log() runs before diffcore_std().
This series restructures the flow so that -L goes through the same log_tree_diff() -> log_tree_diff_flush() path as normal single-parent and merge diffs, then uses that to enable several non-patch diff formats.
Patch 1: revision: move -L setup before output_format-to-diff derivation
Preparatory reorder in setup_revisions(). The -L block sets a default DIFF_FORMAT_PATCH when no format is requested; move it before the derivation of revs->diff from output_format so the default is visible to that check. No behavior change on its own.
Patch 2: line-log: integrate -L output with the standard log-tree pipeline
Rename line_log_print() to line_log_queue_pairs(), stripping it down to only queue pre-computed filepairs. log_tree_diff_flush() handles show_log(), diffcore_std(), and diff_flush(). This fixes pickaxe and --diff-filter suppression, and aligns the commit/diff separator with the rest of log output. Rejects --full-diff, which is not yet supported when filepairs are pre-computed.
Patch 3: line-log: allow non-patch diff formats with -L
Expand the allowlist to accept --raw, --name-only, --name-status, and --summary. These only read filepair metadata already set by the line-log machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call compute_diffstat() on full blob content and would show whole-file statistics rather than range-scoped ones.
Changes since v1:
* Patch 2: use !opt->loginfo return convention in log_tree_diff() to match the existing single-parent and merge codepaths, instead of returning log_tree_diff_flush() directly.
* Patch 2: reword the early-return removal to explicitly tie it to the pipeline change.
* Patch 2: soften --full-diff rejection to "not yet supported".
* Patches 2-3: use test_grep consistently in new tests.
* Patch 2: replace sed | grep pipe with sed > file && test_grep for proper exit status handling.
Michael Montalbo (3):
revision: move -L setup before output_format-to-diff derivation
line-log: integrate -L output with the standard log-tree pipeline
line-log: allow non-patch diff formats with -L
Documentation/line-range-options.adoc | 10 +- line-log.c | 30 ++---- line-log.h | 2 +- log-tree.c | 10 +- revision.c | 24 +++-- t/t4211-line-log.sh | 100 +++++++++++++++--- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 8 files changed, 121 insertions(+), 57 deletions(-)
base-commit: 9f223ef1c026d91c7ac68cc0211bde255dda6199
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2094
Range-diff vs v1:
1: 9633eb62c6 = 1: 9633eb62c6 revision: move -L setup before output_format-to-diff derivation
2: 2d9e0ca015 ! 2: 7acfc5376e line-log: integrate -L output with the standard log-tree pipeline
@@ Commit message
log_tree_diff_flush(), mirroring the diff_tree_oid() + flush
pattern used by the single-parent and merge codepaths.
- - Remove the early return in log_tree_commit() that bypassed
- no_free save/restore, always_show_header, and diff_free().
+ - Remove the early return in log_tree_commit() that is no longer
+ needed now that -L output flows through log_tree_diff() and
+ log_tree_diff_flush(); this restores no_free save/restore,
+ always_show_header, and diff_free() cleanup.
Because show_log() is now deferred until after diffcore_std() inside
log_tree_diff_flush(), pickaxe (-S, -G, --find-object) and
@@ Commit message
log_tree_diff_flush() only emits one for verbose headers. This
matches the rest of log output.
- Also reject --full-diff, which is meaningless with -L: the filepairs
- are pre-computed during the history walk and scoped to tracked paths,
- so there is no tree diff to widen.
+ Also reject --full-diff, which is not yet supported with -L: the
+ filepairs are pre-computed during the history walk and scoped to
+ tracked line ranges, so there is currently no full-tree diff to
+ fall back to for display.
Update tests accordingly.
@@ log-tree.c: static int log_tree_diff(struct rev_info *opt, struct commit *commit
+ if (opt->line_level_traverse) {
+ line_log_queue_pairs(opt, commit);
-+ return log_tree_diff_flush(opt);
++ log_tree_diff_flush(opt);
++ return !opt->loginfo;
+ }
+
parse_commit_or_die(commit);
@@ log-tree.c: int log_tree_commit(struct rev_info *opt, struct commit *commit)
## revision.c ##
@@ revision.c: int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
+ die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
+
if (revs->line_level_traverse &&
- (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))
- die(_("-L does not yet support diff formats besides -p and -s"));
-+ if (revs->line_level_traverse && revs->full_diff)
-+ die(_("-L is not compatible with --full-diff"));
+- (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))
+- die(_("-L does not yet support diff formats besides -p and -s"));
++ (revs->full_diff ||
++ (revs->diffopt.output_format &
++ ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT))))
++ die(_("-L does not yet support the requested diff format"));
if (revs->expand_tabs_in_log < 0)
revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
@@ t/t4211-line-log.sh: test_expect_success '-L with -G filters to diff-text matche
+ test_cmp expect actual
+'
+
-+test_expect_success '--full-diff is not supported with -L' '
++test_expect_success '--full-diff is not yet supported with -L' '
+ test_must_fail git log -L1,24:b.c --full-diff 2>err &&
-+ test_grep "not compatible with --full-diff" err
++ test_grep "does not yet support" err
+'
+
+test_expect_success '-L --oneline has no extra blank line before diff' '
+ git checkout parent-oids &&
+ git log --oneline -L:func2:file.c -1 >actual &&
+ # Oneline header on line 1, diff starts immediately on line 2
-+ sed -n 2p actual | grep "^diff --git"
++ sed -n 2p actual >line2 &&
++ test_grep "^diff --git" line2
+'
+
test_done
3: 06c24b416f ! 3: 10a3d8dde2 line-log: allow non-patch diff formats with -L
@@ Documentation/line-range-options.adoc
## revision.c ##
@@ revision.c: int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
- die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
-
if (revs->line_level_traverse &&
-- (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT)))
-- die(_("-L does not yet support diff formats besides -p and -s"));
-+ (revs->diffopt.output_format &
-+ ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT |
-+ DIFF_FORMAT_RAW | DIFF_FORMAT_NAME |
-+ DIFF_FORMAT_NAME_STATUS | DIFF_FORMAT_SUMMARY)))
-+ die(_("-L does not yet support the requested diff format"));
- if (revs->line_level_traverse && revs->full_diff)
- die(_("-L is not compatible with --full-diff"));
+ (revs->full_diff ||
+ (revs->diffopt.output_format &
+- ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT))))
++ ~(DIFF_FORMAT_PATCH | DIFF_FORMAT_NO_OUTPUT |
++ DIFF_FORMAT_RAW | DIFF_FORMAT_NAME |
++ DIFF_FORMAT_NAME_STATUS | DIFF_FORMAT_SUMMARY))))
+ die(_("-L does not yet support the requested diff format"));
+ if (revs->expand_tabs_in_log < 0)
## t/t4211-line-log.sh ##
@@ t/t4211-line-log.sh: test_expect_success '-p shows the default patch output' '
@@ t/t4211-line-log.sh: test_expect_success '-p shows the default patch output' '
- test_must_fail git log -L1,24:b.c --raw
+test_expect_success '--raw shows mode, oid, status and path' '
+ git log -L1,24:b.c --raw --format= >actual &&
-+ grep "^:100644 100644 [0-9a-f]\{7\} [0-9a-f]\{7\} M b.c$" actual &&
-+ ! grep "^diff --git" actual &&
-+ ! grep "^@@" actual
++ test_grep "^:100644 100644 [0-9a-f]\{7\} [0-9a-f]\{7\} M b.c$" actual &&
++ ! test_grep "^diff --git" actual &&
++ ! test_grep "^@@" actual
+'
+
+test_expect_success '--name-only shows path' '
+ git log -L1,24:b.c --name-only --format= >actual &&
-+ grep "^b.c$" actual &&
-+ ! grep "^diff --git" actual &&
-+ ! grep "^@@" actual
++ test_grep "^b.c$" actual &&
++ ! test_grep "^diff --git" actual &&
++ ! test_grep "^@@" actual
+'
+
+test_expect_success '--name-status shows status and path' '
+ git log -L1,24:b.c --name-status --format= >actual &&
-+ grep "^M b.c$" actual &&
-+ ! grep "^diff --git" actual &&
-+ ! grep "^@@" actual
++ test_grep "^M b.c$" actual &&
++ ! test_grep "^diff --git" actual &&
++ ! test_grep "^@@" actual
+'
+
+test_expect_success '--stat is not yet supported with -L' '
@@ t/t4211-line-log.sh: test_expect_success '-p shows the default patch output' '
test_expect_success 'setup for checking fancy rename following' '
@@ t/t4211-line-log.sh: test_expect_success '-L --oneline has no extra blank line before diff' '
- sed -n 2p actual | grep "^diff --git"
+ test_grep "^diff --git" line2
'
+test_expect_success '--summary shows new file on root commit' '
+ git checkout parent-oids &&
+ git log -L:func2:file.c --summary --format= >actual &&
-+ grep "create mode 100644 file.c" actual
++ test_grep "create mode 100644 file.c" actual
+'
+
test_doneSignature · 1 line
gitgitgadget
PATCH v2 1/3 revision: move -L setup before output_format-to-diff derivation
From: Michael Montalbo <[email protected]>
The line_level_traverse block sets a default DIFF_FORMAT_PATCH when no output format has been explicitly requested. This default must be visible to the "Did the user ask for any diff output?" check that derives revs->diff from revs->diffopt.output_format.
Currently the -L block runs after that derivation, so revs->diff stays 0 when no explicit format is given. This does not matter yet because log_tree_commit() short-circuits into line_log_print() before consulting revs->diff, but the next commit will route -L through the normal log_tree_diff() path, which checks revs->diff.
Move the block above the derivation so the default DIFF_FORMAT_PATCH is in place when revs->diff is computed. No behavior change on its own.
--- revision.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
revision.c +8 -8
@@ -3112,6 +3112,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sobject_context_release (&oc); } +if (revs->line_level_traverse) { +if (want_ancestry (revs)) + revs->limited =1 ; + revs->topo_order =1 ; +if (!revs->diffopt.output_format) + revs->diffopt.output_format =DIFF_FORMAT_PATCH ; + } +/* Did the user ask for any diff output? Run the diff! */ if (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT ) revs->diff =1 ;
@@ -3125,14 +3133,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sif (revs->diffopt.objfind) revs->simplify_history =0 ; -if (revs->line_level_traverse) { -if (want_ancestry (revs)) - revs->limited =1 ; - revs->topo_order =1 ; -if (!revs->diffopt.output_format) - revs->diffopt.output_format =DIFF_FORMAT_PATCH ; - } -if (revs->topo_order && !generation_numbers_enabled (the_repository)) revs->limited =1 ;
Signature · 1 line
gitgitgadget
PATCH v2 2/3 line-log: integrate -L output with the standard log-tree pipeline
In reply to Michael Montalbo via GitGitGadgetFrom: Michael Montalbo <[email protected]>
git log -L has bypassed log_tree_diff() and log_tree_diff_flush() since the feature was introduced, short-circuiting from log_tree_commit() directly into line_log_print(). This skips the no_free save/restore (noted in a NEEDSWORK comment added by f8781bfda3), the always_show_header fallback, show_diff_of_diff(), and diff_free() cleanup.
Restructure so that -L flows through log_tree_diff() -> log_tree_diff_flush(), the same path used by the normal single-parent and merge diff codepaths:
- Rename line_log_print() to line_log_queue_pairs() and strip it down to just queuing pre-computed filepairs. The show_log(), separator, diffcore_std(), and diff_flush() calls are removed since log_tree_diff_flush() handles all of those.
- In log_tree_diff(), call line_log_queue_pairs() then log_tree_diff_flush(), mirroring the diff_tree_oid() + flush pattern used by the single-parent and merge codepaths.
- Remove the early return in log_tree_commit() that is no longer needed now that -L output flows through log_tree_diff() and log_tree_diff_flush(); this restores no_free save/restore, always_show_header, and diff_free() cleanup.
Because show_log() is now deferred until after diffcore_std() inside log_tree_diff_flush(), pickaxe (-S, -G, --find-object) and --diff-filter now properly suppress commits when all pairs are filtered out.
The blank-line separator between commit header and diff changes
slightly: the old code printed one unconditionally, while log_tree_diff_flush() only emits one for verbose headers. This matches the rest of log output.
Also reject --full-diff, which is not yet supported with -L: the filepairs are pre-computed during the history walk and scoped to tracked line ranges, so there is currently no full-tree diff to fall back to for display.
Update tests accordingly.
--- line-log.c | 30 ++++------- line-log.h | 2 +- log-tree.c | 10 ++-- revision.c | 6 ++- t/t4211-line-log.sh | 53 ++++++++++++++----- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 7 files changed, 60 insertions(+), 43 deletions(-)
line-log.c +9 -21
@@ -13,7 +13,6 @@#include "revision.h" #include "xdiff-interface.h" #include "strbuf.h" -#include "log-tree.h" #include "line-log.h" #include "setup.h" #include "strvec.h"
@@ -1004,29 +1003,18 @@ static int process_all_files(struct line_log_data **range_out,return changed; } -int line_log_print (struct rev_info *rev,struct commit *commit) +void line_log_queue_pairs (struct rev_info *rev,struct commit *commit) { -show_log (rev); -if (!(rev->diffopt.output_format &DIFF_FORMAT_NO_OUTPUT )) { -struct line_log_data *range =lookup_line_range (rev, commit); -struct line_log_data *r; -const char *prefix =diff_line_prefix (&rev->diffopt); - -fprintf (rev->diffopt.file,"%s\n" , prefix); - -for (r = range; r; r = r->next) { -if (r->pair) { -struct diff_filepair *p = -diff_filepair_dup (r->pair); - p->line_ranges = &r->ranges; -diff_q (&diff_queued_diff, p); - } - } +struct line_log_data *range =lookup_line_range (rev, commit); +struct line_log_data *r; -diffcore_std (&rev->diffopt); -diff_flush (&rev->diffopt); +for (r = range; r; r = r->next) { +if (r->pair) { +struct diff_filepair *p =diff_filepair_dup (r->pair); + p->line_ranges = &r->ranges; +diff_q (&diff_queued_diff, p); + } } -return 1 ; } static int bloom_filter_check(struct rev_info *rev,
line-log.h +1 -1
@@ -46,7 +46,7 @@ int line_log_filter(struct rev_info *rev);int line_log_process_ranges_arbitrary_commit (struct rev_info *rev,struct commit *commit); -int line_log_print (struct rev_info *rev,struct commit *commit); +void line_log_queue_pairs (struct rev_info *rev,struct commit *commit);void line_log_free (struct rev_info *rev);
log-tree.c +6 -4
@@ -1105,6 +1105,12 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct logif (!all_need_diff && !opt->merges_need_diff)return 0 ; +if (opt->line_level_traverse) { +line_log_queue_pairs (opt, commit); +log_tree_diff_flush (opt); +return !opt->loginfo; + } +parse_commit_or_die (commit); oid =get_commit_tree_oid (commit);
@@ -1179,10 +1185,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit) opt->loginfo = &log; opt->diffopt.no_free =1 ; -/* NEEDSWORK: no restoring of no_free? Why? */ -if (opt->line_level_traverse) -return line_log_print (opt, commit); -if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)fprintf (opt->diffopt.file,"\n%s\n" , opt->break_bar); shown =log_tree_diff (opt, commit, &log);
revision.c +4 -2
@@ -3179,8 +3179,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sdie (_ ("the option '%s' requires '%s'" ),"--grep-reflog" ,"--walk-reflogs" );if (revs->line_level_traverse && - (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT ))) -die (_ ("-L does not yet support diff formats besides -p and -s" )); + (revs->full_diff || + (revs->diffopt.output_format & + ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT )))) +die (_ ("-L does not yet support the requested diff format" ));if (revs->expand_tabs_in_log <0 ) revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
t/t4211-line-log.sh +40 -13
95 lines not on this page · raw message
t/t4211/sha1/expect.parallel-change-f-to-main +0 -1
@@ -5,7 +5,6 @@ Date: Fri Apr 12 16:16:24 2013 +0200 Merge across the rename - commit 6ce3c4ff690136099bb17e1a8766b75764726ea7 Author: Thomas Rast <[email protected]> Date: Thu Feb 28 10:49:50 2013 +0100
t/t4211/sha256/expect.parallel-change-f-to-main +0 -1
@@ -5,7 +5,6 @@ Date: Fri Apr 12 16:16:24 2013 +0200 Merge across the rename - commit 4f7a58195a92c400e28a2354328587f1ff14fb77f5cf894536f17ccbc72931b9 Author: Thomas Rast <[email protected]> Date: Thu Feb 28 10:49:50 2013 +0100
Signature · 1 line
gitgitgadget
PATCH v2 3/3 line-log: allow non-patch diff formats with -L
In reply to Michael Montalbo via GitGitGadgetFrom: Michael Montalbo <[email protected]>
Now that -L flows through log_tree_diff_flush() and diff_flush(), metadata-only diff formats work because they only read filepair fields (status, mode, path, oid) already set on the pre-computed pairs.
Expand the allowlist in setup_revisions() to also accept --raw, --name-only, --name-status, and --summary. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call compute_diffstat() on full blob content and would show whole-file statistics rather than range-scoped ones.
--- Documentation/line-range-options.adoc | 10 +++--- revision.c | 4 ++- t/t4211-line-log.sh | 47 +++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 7 deletions(-)
Documentation/line-range-options.adoc +6 -4
@@ -8,12 +8,14 @@ give zero or one positive revision arguments, and _<start>_ and _<end>_ (or _<funcname>_) must exist in the starting revision. You can specify this option more than once. Implies `--patch`. - Patch output can be suppressed using `--no-patch`, but other diff formats - (namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`, - `--name-only`, `--name-status`, `--check`) are not currently implemented. + Patch output can be suppressed using `--no-patch`. + Non-patch diff formats `--raw`, `--name-only`, `--name-status`, + and `--summary` are supported. Diff stat formats + (`--stat`, `--numstat`, `--shortstat`, `--dirstat`) are not + currently implemented. + Patch formatting options such as `--word-diff`, `--color-moved`, `--no-prefix`, and whitespace options (`-w`, `-b`) are supported, -as are pickaxe options (`-S`, `-G`). +as are pickaxe options (`-S`, `-G`) and `--diff-filter`. + include::line-range-format.adoc[]
revision.c +3 -1
@@ -3181,7 +3181,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sif (revs->line_level_traverse && (revs->full_diff || (revs->diffopt.output_format & - ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT )))) + ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT | +DIFF_FORMAT_RAW |DIFF_FORMAT_NAME | +DIFF_FORMAT_NAME_STATUS |DIFF_FORMAT_SUMMARY ))))die (_ ("-L does not yet support the requested diff format" ));if (revs->expand_tabs_in_log <0 )
t/t4211-line-log.sh +45 -2
57 lines not on this page · raw message
Signature · 1 line
gitgitgadget
Quoted 64 lines · Michael Montalbo via GitGitGadget
Since its introduction, git log -L has short-circuited from
log_tree_commit()into its own output function, bypassinglog_tree_diff()andlog_tree_diff_flush(). This skipsno_freesave/restore,always_show_header,diff_free()cleanup, and means that pickaxe (-S, -G, --find-object) and --diff-filter cannot suppress commits whose pairs are all filtered out, becauseshow_log()runs beforediffcore_std().This series restructures the flow so that -L goes through the same
log_tree_diff()->log_tree_diff_flush()path as normal single-parent and merge diffs, then uses that to enable several non-patch diff formats.Patch 1: revision: move -L setup before output_format-to-diff derivation
Preparatory reorder in
setup_revisions(). The -L block sets a defaultDIFF_FORMAT_PATCHwhen no format is requested; move it before the derivation ofrevs->difffromoutput_formatso the default is visible to that check. No behavior change on its own.Patch 2: line-log: integrate -L output with the standard log-tree pipeline
Rename
line_log_print()toline_log_queue_pairs(), stripping it down to only queue pre-computed filepairs.log_tree_diff_flush()handlesshow_log(),diffcore_std(), anddiff_flush(). This fixes pickaxe and --diff-filter suppression, and aligns the commit/diff separator with the rest of log output. Rejects --full-diff, which is not yet supported when filepairs are pre-computed.Patch 3: line-log: allow non-patch diff formats with -L
Expand the allowlist to accept --raw, --name-only, --name-status, and --summary. These only read filepair metadata already set by the line-log machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call
compute_diffstat()on full blob content and would show whole-file statistics rather than range-scoped ones.Changes since v1:
* Patch 2: use !opt->loginfo return convention in
log_tree_diff()to match the existing single-parent and merge codepaths, instead of returninglog_tree_diff_flush()directly.
* Patch 2: reword the early-return removal to explicitly tie it to the pipeline change.
* Patch 2: soften --full-diff rejection to "not yet supported".
* Patches 2-3: usetest_grepconsistently in new tests.
* Patch 2: replace sed | grep pipe with sed > file &&test_grepfor proper exit status handling.Michael Montalbo (3):
revision: move -L setup before output_format-to-diff derivation
line-log: integrate -L output with the standard log-tree pipeline
line-log: allow non-patch diff formats with -LDocumentation/line-range-options.adoc | 10 +- line-log.c | 30 ++---- line-log.h | 2 +- log-tree.c | 10 +- revision.c | 24 +++-- t/t4211-line-log.sh | 100 +++++++++++++++--- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 8 files changed, 121 insertions(+), 57 deletions(-)
Quoted 6 lines
base-commit:
9f223ef1c026d91c7ac68cc0211bde255dda6199
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2094Range-diff vs v1:
Looks good.
Quoted 108 lines
1: 9633eb62c6 = 1: 9633eb62c6 revision: move -L setup before output_format-to-diff derivation 2: 2d9e0ca015 ! 2: 7acfc5376e line-log: integrate -L output with the standard log-tree pipeline@@ Commit message
log_tree_diff_flush(), mirroring thediff_tree_oid()+ flush pattern used by the single-parent and merge codepaths.- - Remove the early return in
log_tree_commit()that bypassed
-no_freesave/restore,always_show_header, anddiff_free().
+ - Remove the early return inlog_tree_commit()that is no longer
+ needed now that -L output flows throughlog_tree_diff()and
+log_tree_diff_flush(); this restoresno_freesave/restore,
+always_show_header, anddiff_free()cleanup.Because
show_log()is now deferred until afterdiffcore_std()insidelog_tree_diff_flush(), pickaxe (-S, -G, --find-object) and
@@ Commit message
log_tree_diff_flush()only emits one for verbose headers. This matches the rest of log output.- Also reject --full-diff, which is meaningless with -L: the filepairs
- are pre-computed during the history walk and scoped to tracked paths,
- so there is no tree diff to widen.
+ Also reject --full-diff, which is not yet supported with -L: the
+ filepairs are pre-computed during the history walk and scoped to
+ tracked line ranges, so there is currently no full-tree diff to
+ fall back to for display.Update tests accordingly.
@@
log-tree.c: static int log_tree_diff(structrev_info*opt, struct commit *commit+ if (
opt->line_level_traverse) {
+line_log_queue_pairs(opt, commit);
-+ returnlog_tree_diff_flush(opt);
++log_tree_diff_flush(opt);
++ return !opt->loginfo;
+ }
+
parse_commit_or_die(commit);
@@log-tree.c: intlog_tree_commit(struct rev_info *opt, struct commit *commit)##
revision.c##
@@revision.c: int setup_revisions(int argc, const char **argv, structrev_info*revs, struct s
+die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
+
if (revs->line_level_traverse&&
- (revs->diffopt.output_format& ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT)))
-die(_("-L does not yet support diff formats besides -p and -s"));
-+ if (revs->line_level_traverse&&revs->full_diff)
-+die(_("-L is not compatible with --full-diff"));
+- (revs->diffopt.output_format& ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT)))
+-die(_("-L does not yet support diff formats besides -p and -s"));
++ (revs->full_diff||
++ (revs->diffopt.output_format&
++ ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT))))
++die(_("-L does not yet support the requested diff format"));if (
revs->expand_tabs_in_log< 0)
revs->expand_tabs_in_log=revs->expand_tabs_in_log_default;
@@t/t4211-line-log.sh:test_expect_success'-L with -G filters to diff-text matche
+test_cmpexpect actual
+'
+
-+test_expect_success '--full-diff is not supported with -L' ' ++test_expect_success '--full-diff is not yet supported with -L' '
+test_must_failgit log -L1,24:b.c --full-diff 2>err &&
-+test_grep"not compatible with --full-diff" err
++test_grep"does not yet support" err
+'
+
+test_expect_success '-L --oneline has no extra blank line before diff' '
+ git checkout parent-oids &&
+ git log --oneline -L:func2:file.c -1 >actual &&
+ # Oneline header on line 1, diff starts immediately on line 2
-+ sed -n 2p actual | grep "^diff --git"
++ sed -n 2p actual >line2 &&
++test_grep"^diff --git" line2
+'
+
test_done
3:06c24b416f! 3:10a3d8dde2line-log: allow non-patch diff formats with -L
@@Documentation/line-range-options.adoc##
revision.c##
@@revision.c: int setup_revisions(int argc, const char **argv, structrev_info*revs, struct s
-die(_("the option '%s' requires '%s'"), "--grep-reflog", "--walk-reflogs");
-
if (revs->line_level_traverse&&
-- (revs->diffopt.output_format& ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT)))
--die(_("-L does not yet support diff formats besides -p and -s"));
-+ (revs->diffopt.output_format&
-+ ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT|
-+DIFF_FORMAT_RAW|DIFF_FORMAT_NAME|
-+DIFF_FORMAT_NAME_STATUS|DIFF_FORMAT_SUMMARY)))
-+die(_("-L does not yet support the requested diff format"));
- if (revs->line_level_traverse&&revs->full_diff)
-die(_("-L is not compatible with --full-diff"));
+ (revs->full_diff||
+ (revs->diffopt.output_format&
+- ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT))))
++ ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT|
++DIFF_FORMAT_RAW|DIFF_FORMAT_NAME|
++DIFF_FORMAT_NAME_STATUS|DIFF_FORMAT_SUMMARY))))
+die(_("-L does not yet support the requested diff format"));+ if (
revs->expand_tabs_in_log< 0)
Quoted 12 lines
##
t/t4211-line-log.sh##
@@t/t4211-line-log.sh:test_expect_success'-p shows the default patch output' '
@@t/t4211-line-log.sh:test_expect_success'-p shows the default patch output' '
-test_must_failgit log -L1,24:b.c --raw
+test_expect_success '--raw shows mode, oid, status and path' '
+ git log -L1,24:b.c --raw --format= >actual &&
-+ grep "^:100644 100644 [0-9a-f]\{7\} [0-9a-f]\{7\} M b.c$" actual &&
-+ ! grep "^diff --git" actual &&
-+ ! grep "^@@" actual
++test_grep"^:100644 100644 [0-9a-f]\{7\} [0-9a-f]\{7\} M b.c$" actual &&
++ !test_grep"^diff --git" actual &&
++ !test_grep"^@@" actual
I wish we had docs for all the little test helpers… in particular, I think this is supposed to be "test_grep !" ?
Quoted 40 lines
+'
+
+test_expect_success '--name-only shows path' '
+ git log -L1,24:b.c --name-only --format= >actual &&
-+ grep "^b.c$" actual &&
-+ ! grep "^diff --git" actual &&
-+ ! grep "^@@" actual
++ test_grep "^b.c$" actual &&
++ ! test_grep "^diff --git" actual &&
++ ! test_grep "^@@" actual
+'
+
+test_expect_success '--name-status shows status and path' '
+ git log -L1,24:b.c --name-status --format= >actual &&
-+ grep "^M b.c$" actual &&
-+ ! grep "^diff --git" actual &&
-+ ! grep "^@@" actual
++ test_grep "^M b.c$" actual &&
++ ! test_grep "^diff --git" actual &&
++ ! test_grep "^@@" actual
+'
+
+test_expect_success '--stat is not yet supported with -L' '
@@ t/t4211-line-log.sh: test_expect_success '-p shows the default patch output' '
test_expect_success 'setup for checking fancy rename following' '
@@ t/t4211-line-log.sh: test_expect_success '-L --oneline has no extra blank line before diff' '
- sed -n 2p actual | grep "^diff --git"
+ test_grep "^diff --git" line2
'
+test_expect_success '--summary shows new file on root commit' '
+ git checkout parent-oids &&
+ git log -L:func2:file.c --summary --format= >actual &&
-+ grep "create mode 100644 file.c" actual
++ test_grep "create mode 100644 file.c" actual
+'
+
test_doneQuoted 2 lines
--
gitgitgadget
Thanks
Signature · 1 line
D. Ben Knoble
Quoted 5 lines · D. Ben Knoble
Quoted 2 lines
++ ! test_grep "^diff --git" actual && ++ ! test_grep "^@@" actualI wish we had docs for all the little test helpers… in particular, I think this is supposed to be "
test_grep!" ?
Good eyes. I wonder if we teach test-lint to catch these.
Quoted 8 lines · Junio C Hamano
Quoted 5 lines · D. Ben Knoble
Quoted 2 lines
++ ! test_grep "^diff --git" actual && ++ ! test_grep "^@@" actualI wish we had docs for all the little test helpers… in particular, I think this is supposed to be "
test_grep!" ?
Good catch, thank you! Will submit a fix and try to address the lack of docs in a follow-up.
Quoted 2 lines
Good eyes. I wonder if we teach test-lint to catch these.
I had the same thought, and started working on a series that does this and migrates
existing offenders. I should be able to submit it soon.
v3 cover line-log: integrate -L with the standard log output pipeline
In reply to Michael Montalbo via GitGitGadgetSince its introduction, git log -L has short-circuited from log_tree_commit() into its own output function, bypassing log_tree_diff() and log_tree_diff_flush(). This skips no_free save/restore, always_show_header, diff_free() cleanup, and means that pickaxe (-S, -G, --find-object) and --diff-filter cannot suppress commits whose pairs are all filtered out, because show_log() runs before diffcore_std().
This series restructures the flow so that -L goes through the same log_tree_diff() -> log_tree_diff_flush() path as normal single-parent and merge diffs, then uses that to enable several non-patch diff formats.
Patch 1: revision: move -L setup before output_format-to-diff derivation
Preparatory reorder in setup_revisions(). The -L block sets a default DIFF_FORMAT_PATCH when no format is requested; move it before the derivation of revs->diff from output_format so the default is visible to that check. No behavior change on its own.
Patch 2: line-log: integrate -L output with the standard log-tree pipeline
Rename line_log_print() to line_log_queue_pairs(), stripping it down to only queue pre-computed filepairs. log_tree_diff_flush() handles show_log(), diffcore_std(), and diff_flush(). This fixes pickaxe and --diff-filter suppression, and aligns the commit/diff separator with the rest of log output. Rejects --full-diff, which is not yet supported when filepairs are pre-computed.
Patch 3: line-log: allow non-patch diff formats with -L
Expand the allowlist to accept --raw, --name-only, --name-status, and --summary. These only read filepair metadata already set by the line-log machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call compute_diffstat() on full blob content and would show whole-file statistics rather than range-scoped ones.
Changes since v2:
* Switch "! test_grep" to "test_grep !" in tests.
Michael Montalbo (3):
revision: move -L setup before output_format-to-diff derivation
line-log: integrate -L output with the standard log-tree pipeline
line-log: allow non-patch diff formats with -L
Documentation/line-range-options.adoc | 10 +- line-log.c | 30 ++---- line-log.h | 2 +- log-tree.c | 10 +- revision.c | 24 +++-- t/t4211-line-log.sh | 100 +++++++++++++++--- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 8 files changed, 121 insertions(+), 57 deletions(-)
base-commit: 9f223ef1c026d91c7ac68cc0211bde255dda6199
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2094
Range-diff vs v2:
1: 9633eb62c6 = 1: 9633eb62c6 revision: move -L setup before output_format-to-diff derivation
2: 7acfc5376e = 2: 7acfc5376e line-log: integrate -L output with the standard log-tree pipeline
3: 10a3d8dde2 ! 3: ae0b7f3ca8 line-log: allow non-patch diff formats with -L
@@ t/t4211-line-log.sh: test_expect_success '-p shows the default patch output' '
+test_expect_success '--raw shows mode, oid, status and path' '
+ git log -L1,24:b.c --raw --format= >actual &&
+ test_grep "^:100644 100644 [0-9a-f]\{7\} [0-9a-f]\{7\} M b.c$" actual &&
-+ ! test_grep "^diff --git" actual &&
-+ ! test_grep "^@@" actual
++ test_grep ! "^diff --git" actual &&
++ test_grep ! "^@@" actual
+'
+
+test_expect_success '--name-only shows path' '
+ git log -L1,24:b.c --name-only --format= >actual &&
+ test_grep "^b.c$" actual &&
-+ ! test_grep "^diff --git" actual &&
-+ ! test_grep "^@@" actual
++ test_grep ! "^diff --git" actual &&
++ test_grep ! "^@@" actual
+'
+
+test_expect_success '--name-status shows status and path' '
+ git log -L1,24:b.c --name-status --format= >actual &&
+ test_grep "^M b.c$" actual &&
-+ ! test_grep "^diff --git" actual &&
-+ ! test_grep "^@@" actual
++ test_grep ! "^diff --git" actual &&
++ test_grep ! "^@@" actual
+'
+
+test_expect_success '--stat is not yet supported with -L' 'Signature · 1 line
gitgitgadget
PATCH v3 1/3 revision: move -L setup before output_format-to-diff derivation
From: Michael Montalbo <[email protected]>
The line_level_traverse block sets a default DIFF_FORMAT_PATCH when no output format has been explicitly requested. This default must be visible to the "Did the user ask for any diff output?" check that derives revs->diff from revs->diffopt.output_format.
Currently the -L block runs after that derivation, so revs->diff stays 0 when no explicit format is given. This does not matter yet because log_tree_commit() short-circuits into line_log_print() before consulting revs->diff, but the next commit will route -L through the normal log_tree_diff() path, which checks revs->diff.
Move the block above the derivation so the default DIFF_FORMAT_PATCH is in place when revs->diff is computed. No behavior change on its own.
--- revision.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
revision.c +8 -8
@@ -3112,6 +3112,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sobject_context_release (&oc); } +if (revs->line_level_traverse) { +if (want_ancestry (revs)) + revs->limited =1 ; + revs->topo_order =1 ; +if (!revs->diffopt.output_format) + revs->diffopt.output_format =DIFF_FORMAT_PATCH ; + } +/* Did the user ask for any diff output? Run the diff! */ if (revs->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT ) revs->diff =1 ;
@@ -3125,14 +3133,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sif (revs->diffopt.objfind) revs->simplify_history =0 ; -if (revs->line_level_traverse) { -if (want_ancestry (revs)) - revs->limited =1 ; - revs->topo_order =1 ; -if (!revs->diffopt.output_format) - revs->diffopt.output_format =DIFF_FORMAT_PATCH ; - } -if (revs->topo_order && !generation_numbers_enabled (the_repository)) revs->limited =1 ;
Signature · 1 line
gitgitgadget
PATCH v3 2/3 line-log: integrate -L output with the standard log-tree pipeline
In reply to Michael Montalbo via GitGitGadgetFrom: Michael Montalbo <[email protected]>
git log -L has bypassed log_tree_diff() and log_tree_diff_flush() since the feature was introduced, short-circuiting from log_tree_commit() directly into line_log_print(). This skips the no_free save/restore (noted in a NEEDSWORK comment added by f8781bfda3), the always_show_header fallback, show_diff_of_diff(), and diff_free() cleanup.
Restructure so that -L flows through log_tree_diff() -> log_tree_diff_flush(), the same path used by the normal single-parent and merge diff codepaths:
- Rename line_log_print() to line_log_queue_pairs() and strip it down to just queuing pre-computed filepairs. The show_log(), separator, diffcore_std(), and diff_flush() calls are removed since log_tree_diff_flush() handles all of those.
- In log_tree_diff(), call line_log_queue_pairs() then log_tree_diff_flush(), mirroring the diff_tree_oid() + flush pattern used by the single-parent and merge codepaths.
- Remove the early return in log_tree_commit() that is no longer needed now that -L output flows through log_tree_diff() and log_tree_diff_flush(); this restores no_free save/restore, always_show_header, and diff_free() cleanup.
Because show_log() is now deferred until after diffcore_std() inside log_tree_diff_flush(), pickaxe (-S, -G, --find-object) and --diff-filter now properly suppress commits when all pairs are filtered out.
The blank-line separator between commit header and diff changes
slightly: the old code printed one unconditionally, while log_tree_diff_flush() only emits one for verbose headers. This matches the rest of log output.
Also reject --full-diff, which is not yet supported with -L: the filepairs are pre-computed during the history walk and scoped to tracked line ranges, so there is currently no full-tree diff to fall back to for display.
Update tests accordingly.
--- line-log.c | 30 ++++------- line-log.h | 2 +- log-tree.c | 10 ++-- revision.c | 6 ++- t/t4211-line-log.sh | 53 ++++++++++++++----- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 7 files changed, 60 insertions(+), 43 deletions(-)
line-log.c +9 -21
@@ -13,7 +13,6 @@#include "revision.h" #include "xdiff-interface.h" #include "strbuf.h" -#include "log-tree.h" #include "line-log.h" #include "setup.h" #include "strvec.h"
@@ -1004,29 +1003,18 @@ static int process_all_files(struct line_log_data **range_out,return changed; } -int line_log_print (struct rev_info *rev,struct commit *commit) +void line_log_queue_pairs (struct rev_info *rev,struct commit *commit) { -show_log (rev); -if (!(rev->diffopt.output_format &DIFF_FORMAT_NO_OUTPUT )) { -struct line_log_data *range =lookup_line_range (rev, commit); -struct line_log_data *r; -const char *prefix =diff_line_prefix (&rev->diffopt); - -fprintf (rev->diffopt.file,"%s\n" , prefix); - -for (r = range; r; r = r->next) { -if (r->pair) { -struct diff_filepair *p = -diff_filepair_dup (r->pair); - p->line_ranges = &r->ranges; -diff_q (&diff_queued_diff, p); - } - } +struct line_log_data *range =lookup_line_range (rev, commit); +struct line_log_data *r; -diffcore_std (&rev->diffopt); -diff_flush (&rev->diffopt); +for (r = range; r; r = r->next) { +if (r->pair) { +struct diff_filepair *p =diff_filepair_dup (r->pair); + p->line_ranges = &r->ranges; +diff_q (&diff_queued_diff, p); + } } -return 1 ; } static int bloom_filter_check(struct rev_info *rev,
line-log.h +1 -1
@@ -46,7 +46,7 @@ int line_log_filter(struct rev_info *rev);int line_log_process_ranges_arbitrary_commit (struct rev_info *rev,struct commit *commit); -int line_log_print (struct rev_info *rev,struct commit *commit); +void line_log_queue_pairs (struct rev_info *rev,struct commit *commit);void line_log_free (struct rev_info *rev);
log-tree.c +6 -4
@@ -1105,6 +1105,12 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct logif (!all_need_diff && !opt->merges_need_diff)return 0 ; +if (opt->line_level_traverse) { +line_log_queue_pairs (opt, commit); +log_tree_diff_flush (opt); +return !opt->loginfo; + } +parse_commit_or_die (commit); oid =get_commit_tree_oid (commit);
@@ -1179,10 +1185,6 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit) opt->loginfo = &log; opt->diffopt.no_free =1 ; -/* NEEDSWORK: no restoring of no_free? Why? */ -if (opt->line_level_traverse) -return line_log_print (opt, commit); -if (opt->track_linear && !opt->linear && !opt->reverse_output_stage)fprintf (opt->diffopt.file,"\n%s\n" , opt->break_bar); shown =log_tree_diff (opt, commit, &log);
revision.c +4 -2
@@ -3179,8 +3179,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sdie (_ ("the option '%s' requires '%s'" ),"--grep-reflog" ,"--walk-reflogs" );if (revs->line_level_traverse && - (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT ))) -die (_ ("-L does not yet support diff formats besides -p and -s" )); + (revs->full_diff || + (revs->diffopt.output_format & + ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT )))) +die (_ ("-L does not yet support the requested diff format" ));if (revs->expand_tabs_in_log <0 ) revs->expand_tabs_in_log = revs->expand_tabs_in_log_default;
t/t4211-line-log.sh +40 -13
95 lines not on this page · raw message
t/t4211/sha1/expect.parallel-change-f-to-main +0 -1
@@ -5,7 +5,6 @@ Date: Fri Apr 12 16:16:24 2013 +0200 Merge across the rename - commit 6ce3c4ff690136099bb17e1a8766b75764726ea7 Author: Thomas Rast <[email protected]> Date: Thu Feb 28 10:49:50 2013 +0100
t/t4211/sha256/expect.parallel-change-f-to-main +0 -1
@@ -5,7 +5,6 @@ Date: Fri Apr 12 16:16:24 2013 +0200 Merge across the rename - commit 4f7a58195a92c400e28a2354328587f1ff14fb77f5cf894536f17ccbc72931b9 Author: Thomas Rast <[email protected]> Date: Thu Feb 28 10:49:50 2013 +0100
Signature · 1 line
gitgitgadget
PATCH v3 3/3 line-log: allow non-patch diff formats with -L
In reply to Michael Montalbo via GitGitGadgetFrom: Michael Montalbo <[email protected]>
Now that -L flows through log_tree_diff_flush() and diff_flush(), metadata-only diff formats work because they only read filepair fields (status, mode, path, oid) already set on the pre-computed pairs.
Expand the allowlist in setup_revisions() to also accept --raw, --name-only, --name-status, and --summary. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call compute_diffstat() on full blob content and would show whole-file statistics rather than range-scoped ones.
--- Documentation/line-range-options.adoc | 10 +++--- revision.c | 4 ++- t/t4211-line-log.sh | 47 +++++++++++++++++++++++++-- 3 files changed, 54 insertions(+), 7 deletions(-)
Documentation/line-range-options.adoc +6 -4
@@ -8,12 +8,14 @@ give zero or one positive revision arguments, and _<start>_ and _<end>_ (or _<funcname>_) must exist in the starting revision. You can specify this option more than once. Implies `--patch`. - Patch output can be suppressed using `--no-patch`, but other diff formats - (namely `--raw`, `--numstat`, `--shortstat`, `--dirstat`, `--summary`, - `--name-only`, `--name-status`, `--check`) are not currently implemented. + Patch output can be suppressed using `--no-patch`. + Non-patch diff formats `--raw`, `--name-only`, `--name-status`, + and `--summary` are supported. Diff stat formats + (`--stat`, `--numstat`, `--shortstat`, `--dirstat`) are not + currently implemented. + Patch formatting options such as `--word-diff`, `--color-moved`, `--no-prefix`, and whitespace options (`-w`, `-b`) are supported, -as are pickaxe options (`-S`, `-G`). +as are pickaxe options (`-S`, `-G`) and `--diff-filter`. + include::line-range-format.adoc[]
revision.c +3 -1
@@ -3181,7 +3181,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct sif (revs->line_level_traverse && (revs->full_diff || (revs->diffopt.output_format & - ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT )))) + ~(DIFF_FORMAT_PATCH |DIFF_FORMAT_NO_OUTPUT | +DIFF_FORMAT_RAW |DIFF_FORMAT_NAME | +DIFF_FORMAT_NAME_STATUS |DIFF_FORMAT_SUMMARY ))))die (_ ("-L does not yet support the requested diff format" ));if (revs->expand_tabs_in_log <0 )
t/t4211-line-log.sh +45 -2
57 lines not on this page · raw message
Signature · 1 line
gitgitgadget
Quoted 38 lines · Michael Montalbo via GitGitGadget
Since its introduction, git log -L has short-circuited from
log_tree_commit()into its own output function, bypassinglog_tree_diff()andlog_tree_diff_flush(). This skipsno_freesave/restore,always_show_header,diff_free()cleanup, and means that pickaxe (-S, -G, --find-object) and --diff-filter cannot suppress commits whose pairs are all filtered out, becauseshow_log()runs beforediffcore_std().This series restructures the flow so that -L goes through the same
log_tree_diff()->log_tree_diff_flush()path as normal single-parent and merge diffs, then uses that to enable several non-patch diff formats.Patch 1: revision: move -L setup before output_format-to-diff derivation
Preparatory reorder in
setup_revisions(). The -L block sets a defaultDIFF_FORMAT_PATCHwhen no format is requested; move it before the derivation ofrevs->difffromoutput_formatso the default is visible to that check. No behavior change on its own.Patch 2: line-log: integrate -L output with the standard log-tree pipeline
Rename
line_log_print()toline_log_queue_pairs(), stripping it down to only queue pre-computed filepairs.log_tree_diff_flush()handlesshow_log(),diffcore_std(), anddiff_flush(). This fixes pickaxe and --diff-filter suppression, and aligns the commit/diff separator with the rest of log output. Rejects --full-diff, which is not yet supported when filepairs are pre-computed.Patch 3: line-log: allow non-patch diff formats with -L
Expand the allowlist to accept --raw, --name-only, --name-status, and --summary. These only read filepair metadata already set by the line-log machinery. Diff stat formats (--stat, --numstat, --shortstat, --dirstat) remain blocked because they call
compute_diffstat()on full blob content and would show whole-file statistics rather than range-scoped ones.
Quoted 3 lines
Changes since v2:
* Switch "!
test_grep" to "test_grep!" in tests.
Thanks ! I did not read the tests carefully for semantic value, but the rationale and overall code looks good to me as discussed previously.
The range-diff here looks good, too.
Quoted 58 lines
Michael Montalbo (3):
revision: move -L setup before output_format-to-diff derivation
line-log: integrate -L output with the standard log-tree pipeline
line-log: allow non-patch diff formats with -LDocumentation/line-range-options.adoc | 10 +- line-log.c | 30 ++---- line-log.h | 2 +- log-tree.c | 10 +- revision.c | 24 +++-- t/t4211-line-log.sh | 100 +++++++++++++++--- t/t4211/sha1/expect.parallel-change-f-to-main | 1 - .../sha256/expect.parallel-change-f-to-main | 1 - 8 files changed, 121 insertions(+), 57 deletions(-)base-commit:
9f223ef1c026d91c7ac68cc0211bde255dda6199
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2094%2Fmmontalbo%2Fmm%2Fline-log-use-log-tree-diff-flush-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2094/mmontalbo/mm/line-log-use-log-tree-diff-flush-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2094Range-diff vs v2:
1: 9633eb62c6 = 1: 9633eb62c6 revision: move -L setup before output_format-to-diff derivation 2: 7acfc5376e = 2: 7acfc5376e line-log: integrate -L output with the standard log-tree pipeline 3: 10a3d8dde2 ! 3: ae0b7f3ca8 line-log: allow non-patch diff formats with -L @@ t/t4211-line-log.sh: test_expect_success '-p shows the default patch output' ' +test_expect_success '--raw shows mode, oid, status and path' ' + git log -L1,24:b.c --raw --format= >actual && + test_grep "^:100644 100644 [0-9a-f]\{7\} [0-9a-f]\{7\} M b.c$" actual && -+ ! test_grep "^diff --git" actual && -+ ! test_grep "^@@" actual ++ test_grep ! "^diff --git" actual && ++ test_grep ! "^@@" actual +' + +test_expect_success '--name-only shows path' ' + git log -L1,24:b.c --name-only --format= >actual && + test_grep "^b.c$" actual && -+ ! test_grep "^diff --git" actual && -+ ! test_grep "^@@" actual ++ test_grep ! "^diff --git" actual && ++ test_grep ! "^@@" actual +' + +test_expect_success '--name-status shows status and path' ' + git log -L1,24:b.c --name-status --format= >actual && + test_grep "^M b.c$" actual && -+ ! test_grep "^diff --git" actual && -+ ! test_grep "^@@" actual ++ test_grep ! "^diff --git" actual && ++ test_grep ! "^@@" actual +' + +test_expect_success '--stat is not yet supported with -L' '--
gitgitgadget
Quoted 9 lines · Ben Knoble
Quoted 3 lines
Changes since v2:
* Switch "!
test_grep" to "test_grep!" in tests.Thanks ! I did not read the tests carefully for semantic value, but the rationale and overall code looks good to me as discussed previously.
The range-diff here looks good, too.
Thanks, both. Let's mark it for 'next' then.