summaryrefslogtreecommitdiff
path: root/t/t3206-range-diff.sh
AgeCommit message (Collapse)AuthorFilesLines
2019-07-11range-diff: add headers to the outer hunk headerLibravatar Thomas Gummerer1-19/+22
Add the section headers/hunk headers we introduced in the previous commits to the outer diff's hunk headers. This makes it easier to understand which change we are actually looking at. For example an outer hunk header might now look like: @@ Documentation/config/interactive.txt while previously it would have only been @@ which doesn't give a lot of context for the change that follows. For completeness also add section headers for the commit metadata and the commit message, although they are arguably less important. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-11range-diff: add filename to inner diffLibravatar Thomas Gummerer1-6/+10
In a range-diff it's not always clear which file a certain funcname of the inner diff belongs to, because the diff header (or section header as added in a previous commit) is not always visible in the range-diff. Add the filename to the inner diffs header, so it's always visible to users. This also allows us to add the filename + the funcname to the outer diffs hunk headers using a custom userdiff pattern, which will be done in the next commit. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-11range-diff: add section header instead of diff headerLibravatar Thomas Gummerer1-6/+85
Currently range-diff keeps the diff header of the inner diff intact (apart from stripping lines starting with index). This diff header is somewhat useful, especially when files get different names in different ranges. However there is no real need to keep the whole diff header for that. The main reason we currently do that is probably because it is easy to do. Introduce a new range diff hunk header, that's enclosed by "##", similar to how line numbers in diff hunks are enclosed by "@@", and give human readable information of what exactly happened to the file, including the file name. This improves the readability of the range-diff by giving more concise information to the users. For example if a file was renamed in one iteration, but not in another, the diff of the headers would be quite noisy. However the diff of a single line is concise and should be easier to understand. Additionally, this allows us to add these range diff section headers to the outer diffs hunk headers using a custom userdiff pattern, which should help making the range-diff more readable. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-11range-diff: suppress line count in outer diffLibravatar Thomas Gummerer1-8/+8
The line count in the outer diff's hunk headers of a range diff is not all that interesting. It merely shows how far along the inner diff are on both sides. That number is of no use for human readers, and range-diffs are not meant to be machine readable. In a subsequent commit we're going to add some more contextual information such as the filename corresponding to the diff to the hunk headers. Remove the unnecessary information, and just keep the "@@" to indicate that a new hunk of the outer diff is starting. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-11range-diff: don't remove funcname from inner diffLibravatar Thomas Gummerer1-3/+3
When postprocessing the inner diff in range-diff, we currently replace the whole hunk header line with just "@@". This matches how 'git tbdiff' used to handle hunk headers as well. Most likely this is being done because line numbers in the hunk header are not relevant without other changes. They can for example easily change if a range is rebased, and lines are added/removed before a change that we actually care about in our ranges. However it can still be useful to have the function name that 'git diff' extracts as additional context for the change. Note that it is not guaranteed that the hunk header actually shows up in the range-diff, and this change only aims to improve the case where a hunk header would already be included in the final output. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-12-04range-diff: always pass at least minimal diff optionsLibravatar Martin Ågren1-7/+13
Commit d8981c3f88 ("format-patch: do not let its diff-options affect --range-diff", 2018-11-30) taught `show_range_diff()` to accept a NULL-pointer as an indication that it should use its own "reasonable default". That fixed a regression from a5170794 ("Merge branch 'ab/range-diff-no-patch'", 2018-11-18), but unfortunately it introduced a regression of its own. In particular, it means we forget the `file` member of the diff options, so rather than placing a range-diff in the cover-letter, we write it to stdout. In order to fix this, rewrite the two callers adjusted by d8981c3f88 to instead create a "dummy" set of diff options where they only fill in the fields we absolutely require, such as output file and color. Modify and extend the existing tests to try and verify that the right contents end up in the right place. Don't revert `show_range_diff()`, i.e., let it keep accepting NULL. Rather than removing what is dead code and figuring out it isn't actually dead and we've broken 2.20, just leave it for now. [es: retain diff coloring when going to stdout] Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-18Merge branch 'ab/range-diff-no-patch'Libravatar Junio C Hamano1-0/+30
The "--no-patch" option, which can be used to get a high-level overview without the actual line-by-line patch difference shown, of the "range-diff" command was earlier broken, which has been corrected. * ab/range-diff-no-patch: range-diff: make diff option behavior (e.g. --stat) consistent range-diff: fix regression in passing along diff options range-diff doc: add a section about output stability
2018-11-14range-diff: make diff option behavior (e.g. --stat) consistentLibravatar Ævar Arnfjörð Bjarmason1-23/+0
Make the behavior when diff options (e.g. "--stat") are passed consistent with how "diff" behaves. Before 73a834e9e2 ("range-diff: relieve callers of low-level configuration burden", 2018-07-22) running range-diff with "--stat" would produce stat output and the diff output, as opposed to how "diff" behaves where once "--stat" is specified "--patch" also needs to be provided to emit the patch output. As noted in a previous change ("range-diff doc: add a section about output stability", 2018-11-07) the "--stat" output with "range-diff" is useless at the moment. But we should behave consistently with "diff" in anticipation of such output being useful in the future, because it would make for confusing UI if "diff" and "range-diff" behaved differently when it came to how they interpret diff options. The new behavior is also consistent with the existing documentation added in ba931edd28 ("range-diff: populate the man page", 2018-08-13). See "[...]also accepts the regular diff options[...]" in git-range-diff(1). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-12range-diff: fix regression in passing along diff optionsLibravatar Ævar Arnfjörð Bjarmason1-0/+53
In 73a834e9e2 ("range-diff: relieve callers of low-level configuration burden", 2018-07-22) we broke passing down options like --no-patch, --stat etc. Fix that regression, and add a test asserting the pre-73a834e9e2 behavior for some of these diff options. As noted in a change leading up to this ("range-diff doc: add a section about output stability", 2018-11-07) the output is not meant to be stable. So this regression test will likely need to be tweaked once we get a "proper" --stat option. See https://public-inbox.org/git/nycvar.QRO.7.76.6.1811071202480.39@tvgsbejvaqbjf.bet/ for a further explanation of the regression. The fix here is not the same as in Johannes's on-list patch, for reasons that'll be explained in a follow-up commit. The quoting of "EOF" here mirrors that of an earlier test. Perhaps that should be fixed, but let's leave that up to a later cleanup change. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-10-25range-diff: allow to diff files regardless of submodule configLibravatar Lucas De Marchi1-0/+29
If we have `submodule.diff = log' in the configuration file or `--submodule=log' is given as argument, range-diff fails to compare both diffs and we only get the following output: Submodule a 0000000...0000000 (new submodule) Even if the repository doesn't have any submodule. That's because the mode in diff_filespec is not correct and when flushing the diff, down in builtin_diff() we will enter the condition: if (o->submodule_format == DIFF_SUBMODULE_LOG && (!one->mode || S_ISGITLINK(one->mode)) && (!two->mode || S_ISGITLINK(two->mode))) { show_submodule_summary(o, one->path ? one->path : two->path, &one->oid, &two->oid, two->dirty_submodule); return; It turns out that S_ISGITLINK will return true (mode == 0160000 here). Similar thing happens if submodule.diff is "diff". Do like it's done in grep.c when calling fill_filespec() and force it to be recognized as a file by adding S_IFREG to the mode. Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-24Merge branch 'tg/range-diff-corner-case-fix'Libravatar Junio C Hamano1-0/+5
Recently added "range-diff" had a corner-case bug to cause it segfault, which has been corrected. * tg/range-diff-corner-case-fix: linear-assignment: fix potential out of bounds memory access
2018-09-24Merge branch 'ds/format-patch-range-diff-test'Libravatar Junio C Hamano1-0/+5
* ds/format-patch-range-diff-test: t3206-range-diff.sh: cover single-patch case
2018-09-17Merge branch 'es/format-patch-rangediff'Libravatar Junio C Hamano1-0/+12
"git format-patch" learned a new "--range-diff" option to explain the difference between this version and the previous attempt in the cover letter (or after the tree-dashes as a comment). * es/format-patch-rangediff: format-patch: allow --range-diff to apply to a lone-patch format-patch: add --creation-factor tweak for --range-diff format-patch: teach --range-diff to respect -v/--reroll-count format-patch: extend --range-diff to accept revision range format-patch: add --range-diff option to embed diff in cover letter range-diff: relieve callers of low-level configuration burden range-diff: publish default creation factor range-diff: respect diff_option.file rather than assuming 'stdout'
2018-09-14linear-assignment: fix potential out of bounds memory accessLibravatar Thomas Gummerer1-0/+5
Currently the 'compute_assignment()' function may read memory out of bounds, even if used correctly. Namely this happens when we only have one column. In that case we try to calculate the initial minimum cost using '!j1' as column in the reduction transfer code. That in turn causes us to try and get the cost from column 1 in the cost matrix, which does not exist, and thus results in an out of bounds memory read. In the original paper [1], the example code initializes that minimum cost to "infinite". We could emulate something similar by setting the minimum cost to INT_MAX, which would result in the same minimum cost as the current algorithm, as we'd always go into the if condition at least once, except when we only have one column, and column_count thus equals 1. If column_count does equal 1, the condition in the loop would always be false, and we'd end up with a minimum of INT_MAX, which may lead to integer overflows later in the algorithm. For a column count of 1, we however do not even really need to go through the whole algorithm. A column count of 1 means that there's no possible assignments, and we can just zero out the column2row and row2column arrays, and return early from the function, while keeping the reduction transfer part of the function the same as it is currently. Another solution would be to just not call the 'compute_assignment()' function from the range diff code in this case, however it's better to make the compute_assignment function more robust, so future callers don't run into this potential problem. Note that the test only fails under valgrind on Linux, but the same command has been reported to segfault on Mac OS. [1]: Jonker, R., & Volgenant, A. (1987). A shortest augmenting path algorithm for dense and sparse linear assignment problems. Computing, 38(4), 325–340. Reported-by: ryenus <ryenus@gmail.com> Helped-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-12t3206-range-diff.sh: cover single-patch caseLibravatar Derrick Stolee1-0/+5
The commit 40ce4160 "format-patch: allow --range-diff to apply to a lone-patch" added the ability to see a range-diff as commentary after the commit message of a single patch series (i.e. [PATCH] instead of [PATCH X/N]). However, this functionality was not covered by a test case. Add a simple test case that checks that a range-diff is written as commentary to the patch. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-20range-diff: indent special lines as contextLibravatar Stefan Beller1-6/+6
The range-diff coloring is a bit fuzzy when it comes to special lines of a diff, such as indicating new and old files with +++ and ---, as it would pickup the first character and interpret it for its coloring, which seems annoying as in regular diffs, these lines are colored bold via DIFF_METAINFO. By indenting these lines by a white space, they will be treated as context which is much more useful, an example [1] on the range diff series itself: [...] + diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt + new file mode 100644 + --- /dev/null + +++ b/Documentation/git-range-diff.txt +@@ ++git-range-diff(1) [...] + diff --git a/Makefile b/Makefile --- a/Makefile +++ b/Makefile [...] The first lines that introduce the new file for the man page will have the '+' sign colored and the rest of the line will be bold. The later lines that indicate a change to the Makefile will be treated as context both in the outer and inner diff, such that those lines stay regular color. [1] ./git-range-diff pr-1/dscho/branch-diff-v3...pr-1/dscho/branch-diff-v4 These tags are found at https://github.com/gitgitgadget/git Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-14format-patch: extend --range-diff to accept revision rangeLibravatar Eric Sunshine1-1/+1
When submitting a revised a patch series, the --range-diff option embeds a range-diff in the cover letter showing changes since the previous version of the patch series. The argument to --range-diff is a simple revision naming the tip of the previous series, which works fine if the previous and current versions of the patch series share a common base. However, it fails if the revision ranges of the old and new versions of the series are disjoint. To address this shortcoming, extend --range-diff to also accept an explicit revision range for the previous series. For example: git format-patch --cover-letter --range-diff=v1~3..v1 -3 v2 Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-14format-patch: add --range-diff option to embed diff in cover letterLibravatar Eric Sunshine1-0/+12
When submitting a revised version of a patch series, it can be helpful (to reviewers) to include a summary of changes since the previous attempt in the form of a range-diff, however, doing so involves manually copy/pasting the diff into the cover letter. Add a --range-diff option to automate this process. The argument to --range-diff specifies the tip of the previous attempt against which to generate the range-diff. For example: git format-patch --cover-letter --range-diff=v1 -3 v2 (At this stage, the previous attempt and the patch series being formatted must share a common base, however, a subsequent enhancement will make it possible to specify an explicit revision range for the previous attempt.) Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-14diff: use emit_line_0 once per lineLibravatar Stefan Beller1-1/+1
All lines that use emit_line_0 multiple times per line, are combined into a single call to emit_line_0, making use of the 'set' argument. We gain a little efficiency here, as we can omit emission of color and accompanying reset if 'len == 0'. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-14t3206: add color test for range-diff --dual-colorLibravatar Stefan Beller1-0/+39
The 'expect'ed outcome has been taken by running the 'range-diff | decode'. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-13range-diff: add testsLibravatar Thomas Rast1-0/+145
These are essentially lifted from https://github.com/trast/tbdiff, with light touch-ups to account for the command now being named `git range-diff`. Apart from renaming `tbdiff` to `range-diff`, only one test case needed to be adjusted: 11 - 'changed message'. The underlying reason it had to be adjusted is that diff generation is sometimes ambiguous. In this case, a comment line and an empty line are added, but it is ambiguous whether they were added after the existing empty line, or whether an empty line and the comment line are added *before* the existing empty line. And apparently xdiff picks a different option here than Python's difflib. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>