Commit graph

222 commits

Author SHA1 Message Date
Junio C Hamano 706b0b5e8d Merge branch 'es/diff-color-moved-fix'
One of the "diff --color-moved" mode "dimmed_zebra" that was named
in an unusual way has been deprecated and replaced by
"dimmed-zebra".

* es/diff-color-moved-fix:
  diff: --color-moved: rename "dimmed_zebra" to "dimmed-zebra"
2018-08-15 15:08:22 -07:00
Junio C Hamano a81575aa91 Merge branch 'sb/diff-color-move-more'
"git diff --color-moved" feature has further been tweaked.

* sb/diff-color-move-more:
  diff.c: offer config option to control ws handling in move detection
  diff.c: add white space mode to move detection that allows indent changes
  diff.c: factor advance_or_nullify out of mark_color_as_moved
  diff.c: decouple white space treatment from move detection algorithm
  diff.c: add a blocks mode for moved code detection
  diff.c: adjust hash function signature to match hashmap expectation
  diff.c: do not pass diff options as keydata to hashmap
  t4015: avoid git as a pipe input
  xdiff/xdiffi.c: remove unneeded function declarations
  xdiff/xdiff.h: remove unused flags
2018-08-02 15:30:40 -07:00
Eric Sunshine e3f2f5f9cd diff: --color-moved: rename "dimmed_zebra" to "dimmed-zebra"
The --color-moved "dimmed_zebra" mode (with an underscore) is an
anachronism. Most options and modes are hyphenated. It is more difficult
to type and somewhat more difficult to read than those which are
hyphenated. Therefore, rename it to "dimmed-zebra", and nominally
deprecate "dimmed_zebra".

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-25 14:23:52 -07:00
Stefan Beller 626c0b5d39 diff.c: offer config option to control ws handling in move detection
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-19 12:02:54 -07:00
Stefan Beller ca1f4ae4df diff.c: add white space mode to move detection that allows indent changes
The option of --color-moved has proven to be useful as observed on the
mailing list. However when refactoring sometimes the indentation changes,
for example when partitioning a functions into smaller helper functions
the code usually mostly moved around except for a decrease in indentation.

To just review the moved code ignoring the change in indentation, a mode
to ignore spaces in the move detection as implemented in a previous patch
would be enough.  However the whole move coloring as motivated in commit
2e2d5ac (diff.c: color moved lines differently, 2017-06-30), brought
up the notion of the reviewer being able to trust the move of a "block".

As there are languages such as python, which depend on proper relative
indentation for the control flow of the program, ignoring any white space
change in a block would not uphold the promises of 2e2d5ac that allows
reviewers to pay less attention to the inside of a block, as inside
the reviewer wants to assume the same program flow.

This new mode of white space ignorance will take this into account and will
only allow the same white space changes per line in each block. This patch
even allows only for the same change at the beginning of the lines.

As this is a white space mode, it is made exclusive to other white space
modes in the move detection.

This patch brings some challenges, related to the detection of blocks.
We need a wide net to catch the possible moved lines, but then need to
narrow down to check if the blocks are still intact. Consider this
example (ignoring block sizes):

 - A
 - B
 - C
 +    A
 +    B
 +    C

At the beginning of a block when checking if there is a counterpart
for A, we have to ignore all space changes. However at the following
lines we have to check if the indent change stayed the same.

Checking if the indentation change did stay the same, is done by computing
the indentation change by the difference in line length, and then assume
the change is only in the beginning of the longer line, the common tail
is the same. That is why the test contains lines like:

 - <TAB> A
 ...
 + A <TAB>
 ...

As the first line starting a block is caught using a compare function that
ignores white spaces unlike the rest of the block, where the white space
delta is taken into account for the comparison, we also have to think about
the following situation:

 - A
 - B
 -   A
 -   B
 +    A
 +    B
 +      A
 +      B

When checking if the first A (both in the + and - lines) is a start of
a block, we have to check all 'A' and record all the white space deltas
such that we can find the example above to be just one block that is
indented.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-19 12:02:54 -07:00
Stefan Beller b3095712f9 diff.c: decouple white space treatment from move detection algorithm
In the original implementation of the move detection logic the choice for
ignoring white space changes is the same for the move detection as it is
for the regular diff.  Some cases came up where different treatment would
have been nice.

Allow the user to specify that white space should be ignored differently
during detection of moved lines than during generation of added and removed
lines. This is done by providing analogs to the --ignore-space-at-eol,
-b, and -w options by introducing the option --color-moved-ws=<modes>
with the modes named "ignore-space-at-eol", "ignore-space-change" and
"ignore-all-space", which is used only during the move detection phase.

As we change the default, we'll adjust the tests.

For now we do not infer any options to treat white spaces in the move
detection from the generic white space options given to diff.
This can be tuned later to reasonable default.

As we plan on adding more white space related options in a later patch,
that interferes with the current white space options, use a flag field
and clamp it down to  XDF_WHITESPACE_FLAGS, as that (a) allows to easily
check at parse time if we give invalid combinations and (b) can reuse
parts of this patch.

By having the white space treatment in its own option, we'll also
make it easier for a later patch to have an config option for
spaces in the move detection.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-17 11:25:31 -07:00
Stefan Beller 51da15eb23 diff.c: add a blocks mode for moved code detection
The new "blocks" mode provides a middle ground between plain and zebra.
It is as intuitive (few colors) as plain, but still has the requirement
for a minimum of lines/characters to count a block as moved.

Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
 (https://public-inbox.org/git/87o9j0uljo.fsf@evledraar.gmail.com/)
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-17 11:25:31 -07:00
Karthikeyan Singaravelan 1f2abe68d0 doc: fix typos in documentation and release notes
Signed-off-by: Karthikeyan Singaravelan <tir.karthi@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-19 09:01:12 -07:00
Junio C Hamano d676cc512a Merge branch 'rd/diff-options-typofix'
Typofix.

* rd/diff-options-typofix:
  diff-options.txt: fix minor typos, font inconsistencies, in docs
2018-06-18 10:18:41 -07:00
Robert P. J. Day 7eedad15df diff-options.txt: fix minor typos, font inconsistencies, in docs
Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-11 13:11:09 -07:00
Junio C Hamano cb6462fe74 Merge branch 'en/doc-typoes'
Docfix.

* en/doc-typoes:
  Documentation: normalize spelling of 'normalised'
  Documentation: fix several one-character-off spelling errors
2018-04-25 13:28:58 +09:00
Elijah Newren c30d4f1b84 Documentation: fix several one-character-off spelling errors
Signed-off-by: Elijah Newren <newren@gmail.com>
Reviewed-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-09 14:15:02 +09:00
Junio C Hamano 868f7d2338 Merge branch 'nd/diff-stat-with-summary'
"git diff" and friends learned "--compact-summary" that shows the
information usually given with the "--summary" option on the same
line as the diffstat output of the "--stat" option (which saves
vertical space and keeps info on a single path at the same place).

* nd/diff-stat-with-summary:
  diff: add --compact-summary
  diff.c: refactor pprint_rename() to use strbuf
2018-03-14 12:01:02 -07:00
Nguyễn Thái Ngọc Duy ddf88fa616 diff: add --compact-summary
Certain information is currently shown with --summary, but when used
in combination with --stat it's a bit hard to read since info of the
same file is in two places (--stat and --summary).

On top of that, commits that add or remove files double the number of
display lines, which could be a lot if you add or remove a lot of
files.

--compact-summary embeds most of --summary back in --stat in the
little space between the file name part and the graph line, e.g. with
commit 0433d533f1:

   Documentation/merge-config.txt         |  4 +
   builtin/merge.c                        |  2 +
   ...-pull-verify-signatures.sh (new +x) | 81 ++++++++++++++
   t/t7612-merge-verify-signatures.sh     | 45 ++++++++
   4 files changed, 132 insertions(+)

It helps both condensing information and saving some text
space. What's new in diffstat is:

- A new 0644 file is shown as (new)
- A new 0755 file is shown as (new +x)
- A new symlink is shown as (new +l)
- A deleted file is shown as (gone)
- A mode change adding executable bit is shown as (mode +x)
- A mode change removing it is shown as (mode -x)

Note that --compact-summary does not contain all the information
--summary provides. Rewrite percentage is not shown but it could be
added later, like R50% or C20%.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-27 15:22:47 -08:00
Junio C Hamano c0d75f0e2e Merge branch 'sb/diff-blobfind-pickaxe'
"diff" family of commands learned "--find-object=<object-id>" option
to limit the findings to changes that involve the named object.

* sb/diff-blobfind-pickaxe:
  diff: use HAS_MULTI_BITS instead of counting bits manually
  diff: properly error out when combining multiple pickaxe options
  diffcore: add a pickaxe option to find a specific blob
  diff: introduce DIFF_PICKAXE_KINDS_MASK
  diff: migrate diff_flags.pickaxe_ignore_case to a pickaxe_opts bit
  diff.h: make pickaxe_opts an unsigned bit field
2018-01-23 13:16:37 -08:00
Junio C Hamano 6366dd9000 Merge branch 'jk/doc-diff-options'
Doc update.

* jk/doc-diff-options:
  docs/diff-options: clarify scope of diff-filter types
2018-01-09 14:32:57 -08:00
Stefan Beller 15af58c1ad diffcore: add a pickaxe option to find a specific blob
Sometimes users are given a hash of an object and they want to
identify it further (ex.: Use verify-pack to find the largest blobs,
but what are these? or [1])

One might be tempted to extend git-describe to also work with blobs,
such that `git describe <blob-id>` gives a description as
'<commit-ish>:<path>'.  This was implemented at [2]; as seen by the sheer
number of responses (>110), it turns out this is tricky to get right.
The hard part to get right is picking the correct 'commit-ish' as that
could be the commit that (re-)introduced the blob or the blob that
removed the blob; the blob could exist in different branches.

Junio hinted at a different approach of solving this problem, which this
patch implements. Teach the diff machinery another flag for restricting
the information to what is shown. For example:

    $ ./git log --oneline --find-object=v2.0.0:Makefile
    b2feb64309 Revert the whole "ask curl-config" topic for now
    47fbfded53 i18n: only extract comments marked with "TRANSLATORS:"

we observe that the Makefile as shipped with 2.0 was appeared in
v1.9.2-471-g47fbfded53 and in v2.0.0-rc1-5-gb2feb6430b.  The
reason why these commits both occur prior to v2.0.0 are evil
merges that are not found using this new mechanism.

[1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob
[2] https://public-inbox.org/git/20171028004419.10139-1-sbeller@google.com/

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-04 15:02:40 -08:00
Jeff King 46af107bde docs/diff-options: clarify scope of diff-filter types
The same document for "--diff-filter" is included by many
programs in the diff family. Because it mentions all
possible types (added, removed, etc), this may imply to the
reader that all types can be generated by a particular
command. But this isn't necessarily the case; "diff-files"
cannot generally produce an "Added" entry, since the diff is
limited to what is already in the index.

Let's make it clear that the list here is the full one, and
does not imply anything about what a particular invocation
may produce.

Note that conditionally including items (e.g., omitting
"Added" in the git-diff-files manpage) isn't the right
solution here for two reasons:

  - The problem isn't diff-files, but doing an index to
    working tree diff. "git diff" can do the same diff, but
    also has other modes where "Added" does show up.

  - The direction of the diff matters. Doing "diff-files -R"
    can get you Added entries (but not Deleted ones).

So it's best just to explain that the set of available types
depends on the specific diff invocation.

Reported-by: John Cheng <johnlicheng@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-04 11:09:47 -08:00
Jonathan Tan 2477ab2ea8 diff: support anchoring line(s)
Teach diff a new algorithm, one that attempts to prevent user-specified
lines from appearing as a deletion or addition in the end result. The
end user can use this by specifying "--anchored=<text>" one or more
times when using Git commands like "diff" and "show".

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-28 10:40:04 +09:00
Junio C Hamano 10f65c239a Merge branch 'jc/ignore-cr-at-eol'
The "diff" family of commands learned to ignore differences in
carriage return at the end of line.

* jc/ignore-cr-at-eol:
  diff: --ignore-cr-at-eol
  xdiff: reassign xpparm_t.flags bits
2017-11-27 11:06:31 +09:00
Junio C Hamano da2b4ee388 Merge branch 'cn/diff-indent-no-longer-is-experimental' into maint
Doc update.

* cn/diff-indent-no-longer-is-experimental:
  diff: --indent-heuristic is no longer experimental
2017-11-15 12:05:04 +09:00
Junio C Hamano e9282f02b2 diff: --ignore-cr-at-eol
A new option --ignore-cr-at-eol tells the diff machinery to treat a
carriage-return at the end of a (complete) line as if it does not
exist.

Just like other "--ignore-*" options to ignore various kinds of
whitespace differences, this will help reviewing the real changes
you made without getting distracted by spurious CRLF<->LF conversion
made by your editor program.

Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
[jch: squashed in command line completion by Dscho]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-08 10:05:27 +09:00
Junio C Hamano 662ac3b3a8 Merge branch 'cn/diff-indent-no-longer-is-experimental'
Doc update.

* cn/diff-indent-no-longer-is-experimental:
  diff: --indent-heuristic is no longer experimental
2017-11-06 13:11:27 +09:00
Carlos Martín Nieto bab76141da diff: --indent-heuristic is no longer experimental
This heuristic has been the default since 2.14 so we should not confuse our
users by saying that it's experimental and off by default.

Signed-off-by: Carlos Martín Nieto <cmn@dwim.me>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-02 14:51:24 +09:00
Junio C Hamano 0b96358479 Merge branch 'jt/diff-color-move-fix'
A handful of bugfixes and an improvement to "diff --color-moved".

* jt/diff-color-move-fix:
  diff: define block by number of alphanumeric chars
  diff: respect MIN_BLOCK_LENGTH for last block
  diff: avoid redundantly clearing a flag
2017-08-26 22:55:04 -07:00
Junio C Hamano b6c4058f97 Merge branch 'sb/diff-color-move'
"git diff" has been taught to optionally paint new lines that are
the same as deleted lines elsewhere differently from genuinely new
lines.

* sb/diff-color-move: (25 commits)
  diff: document the new --color-moved setting
  diff.c: add dimming to moved line detection
  diff.c: color moved lines differently, plain mode
  diff.c: color moved lines differently
  diff.c: buffer all output if asked to
  diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARY
  diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEP
  diff.c: convert word diffing to use emit_diff_symbol
  diff.c: convert show_stats to use emit_diff_symbol
  diff.c: convert emit_binary_diff_body to use emit_diff_symbol
  submodule.c: migrate diff output to use emit_diff_symbol
  diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFF
  diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILES
  diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADER
  diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIR_{PLUS, MINUS}
  diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETE
  diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS[_PORCELAIN]
  diff.c: migrate emit_line_checked to use emit_diff_symbol
  diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOF
  diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFO
  ...
2017-08-26 22:55:03 -07:00
Junio C Hamano b55b936038 Merge branch 'ah/doc-wserrorhighlight' into maint
Doc update.

* ah/doc-wserrorhighlight:
  doc: add missing values "none" and "default" for diff.wsErrorHighlight
2017-08-23 14:33:51 -07:00
Jonathan Tan f0b8fb6e59 diff: define block by number of alphanumeric chars
The existing behavior of diff --color-moved=zebra does not define the
minimum size of a block at all, instead relying on a heuristic applied
later to filter out sets of adjacent moved lines that are shorter than 3
lines long. This can be confusing, because a block could thus be colored
as moved at the source but not at the destination (or vice versa),
depending on its neighbors.

Instead, teach diff that the minimum size of a block is 20 alphanumeric
characters, the same heuristic used by "git blame". This allows diff to
still exclude uninteresting lines appearing on their own (such as those
solely consisting of one or a few closing braces), as was the intention
of the adjacent-moved-line heuristic.

This requires a change in some tests in that some of their lines are no
longer considered to be part of a block, because they are too short.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-16 11:44:00 -07:00
Junio C Hamano 4c244c25f0 Merge branch 'ah/doc-wserrorhighlight'
Doc update.

* ah/doc-wserrorhighlight:
  doc: add missing values "none" and "default" for diff.wsErrorHighlight
2017-08-11 13:27:04 -07:00
Anthony Sottile bc9b7e207f diff-options doc: grammar fix
Signed-off-by: Anthony Sottile <asottile@umich.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-31 09:57:12 -07:00
Andreas Heiduk c0bb6d9cef doc: add missing values "none" and "default" for diff.wsErrorHighlight
The values have eluded documentation so far. While at it streamline
the wording by grouping relevant parts together.

Signed-off-by: Andreas Heiduk <asheiduk@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-25 14:30:49 -07:00
Stefan Beller 61e89eaae8 diff: document the new --color-moved setting
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-30 13:59:42 -07:00
Andreas Heiduk 860cd699c2 Documentation: improve description for core.quotePath
Linking the description for pathname quoting to the configuration
variable "core.quotePath" removes inconstistent and incomplete
sections while also giving two hints how to deal with it: Either with
"-c core.quotePath=false" or with "-z".

Signed-off-by: Andreas Heiduk <asheiduk@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-02 11:40:51 -08:00
Junio C Hamano 831bd5e775 Merge branch 'rh/diff-orderfile-doc'
Documentation fix.

* rh/diff-orderfile-doc:
  diff: document the format of the -O (diff.orderFile) file
  diff: document behavior of relative diff.orderFile
2017-01-23 15:59:23 -08:00
Richard Hansen 874444b704 diff: document the format of the -O (diff.orderFile) file
Signed-off-by: Richard Hansen <hansenr@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-15 16:42:16 -08:00
Vegard Nossum c488867793 diff: add interhunk context config option
The --inter-hunk-context= option was added in commit 6d0e674a57
("diff: add option to show context between close hunks"). This patch
allows configuring a default for this option.

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-12 12:55:43 -08:00
Junio C Hamano 650360210a Merge branch 'nd/ita-empty-commit'
When new paths were added by "git add -N" to the index, it was
enough to circumvent the check by "git commit" to refrain from
making an empty commit without "--allow-empty".  The same logic
prevented "git status" to show such a path as "new file" in the
"Changes not staged for commit" section.

* nd/ita-empty-commit:
  commit: don't be fooled by ita entries when creating initial commit
  commit: fix empty commit creation when there's no changes but ita entries
  diff: add --ita-[in]visible-in-index
  diff-lib: allow ita entries treated as "not yet exist in index"
2016-10-27 14:58:50 -07:00
Junio C Hamano e5272d304a Merge branch 'jc/ws-error-highlight'
"git diff/log --ws-error-highlight=<kind>" lacked the corresponding
configuration variable to set it by default.

* jc/ws-error-highlight:
  diff: introduce diff.wsErrorHighlight option
  diff.c: move ws-error-highlight parsing helpers up
  diff.c: refactor parse_ws_error_highlight()
  t4015: split out the "setup" part of ws-error-highlight test
2016-10-26 13:14:43 -07:00
Nguyễn Thái Ngọc Duy b42b451919 diff: add --ita-[in]visible-in-index
The option --ita-invisible-in-index exposes the "ita_invisible_in_index"
diff flag to outside to allow easier experimentation with this new mode.
The "plan" is to make --ita-invisible-in-index default to keep consistent
behavior with 'status' and 'commit', but a bunch other commands like
'apply', 'merge', 'reset'.... need to be taken into consideration as well.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-24 10:47:51 -07:00
Junio C Hamano a17505f262 diff: introduce diff.wsErrorHighlight option
With the preparatory steps, it has become trivial to teach the
system a new diff.wsErrorHighlight configuration that gives the
default value for --ws-error-highlight command line option.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-04 15:49:05 -07:00
Junio C Hamano b7af6ae5cf Merge branch 'mh/diff-indent-heuristic'
Output from "git diff" can be made easier to read by selecting
which lines are common and which lines are added/deleted
intelligently when the lines before and after the changed section
are the same.  A command line option is added to help with the
experiment to find a good heuristics.

* mh/diff-indent-heuristic:
  blame: honor the diff heuristic options and config
  parse-options: add parse_opt_unknown_cb()
  diff: improve positioning of add/delete blocks in diffs
  xdl_change_compact(): introduce the concept of a change group
  recs_match(): take two xrecord_t pointers as arguments
  is_blank_line(): take a single xrecord_t as argument
  xdl_change_compact(): only use heuristic if group can't be matched
  xdl_change_compact(): fix compaction heuristic to adjust ixo
2016-09-26 16:09:16 -07:00
Michael Haggerty 5b162879e9 blame: honor the diff heuristic options and config
Teach "git blame" and "git annotate" the --compaction-heuristic and
--indent-heuristic options that are now supported by "git diff".

Also teach them to honor the `diff.compactionHeuristic` and
`diff.indentHeuristic` configuration options.

It would be conceivable to introduce separate configuration options for
"blame" and "annotate"; for example `blame.compactionHeuristic` and
`blame.indentHeuristic`. But it would be confusing to users if blame
output is inconsistent with diff output, so it makes more sense for them
to respect the same configuration.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-19 10:25:11 -07:00
Michael Haggerty 433860f3d0 diff: improve positioning of add/delete blocks in diffs
Some groups of added/deleted lines in diffs can be slid up or down,
because lines at the edges of the group are not unique. Picking good
shifts for such groups is not a matter of correctness but definitely has
a big effect on aesthetics. For example, consider the following two
diffs. The first is what standard Git emits:

    --- a/9c572b21dd090a1e5c5bb397053bf8043ffe7fb4:git-send-email.perl
    +++ b/6dcfa306f2b67b733a7eb2d7ded1bc9987809edb:git-send-email.perl
    @@ -231,6 +231,9 @@ if (!defined $initial_reply_to && $prompting) {
     }

     if (!$smtp_server) {
    +       $smtp_server = $repo->config('sendemail.smtpserver');
    +}
    +if (!$smtp_server) {
            foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
                    if (-x $_) {
                            $smtp_server = $_;

The following diff is equivalent, but is obviously preferable from an
aesthetic point of view:

    --- a/9c572b21dd090a1e5c5bb397053bf8043ffe7fb4:git-send-email.perl
    +++ b/6dcfa306f2b67b733a7eb2d7ded1bc9987809edb:git-send-email.perl
    @@ -230,6 +230,9 @@ if (!defined $initial_reply_to && $prompting) {
            $initial_reply_to =~ s/(^\s+|\s+$)//g;
     }

    +if (!$smtp_server) {
    +       $smtp_server = $repo->config('sendemail.smtpserver');
    +}
     if (!$smtp_server) {
            foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) {
                    if (-x $_) {

This patch teaches Git to pick better positions for such "diff sliders"
using heuristics that take the positions of nearby blank lines and the
indentation of nearby lines into account.

The existing Git code basically always shifts such "sliders" as far down
in the file as possible. The only exception is when the slider can be
aligned with a group of changed lines in the other file, in which case
Git favors depicting the change as one add+delete block rather than one
add and a slightly offset delete block. This naive algorithm often
yields ugly diffs.

Commit d634d61ed6 improved the situation somewhat by preferring to
position add/delete groups to make their last line a blank line, when
that is possible. This heuristic does more good than harm, but (1) it
can only help if there are blank lines in the right places, and (2)
always picks the last blank line, even if there are others that might be
better. The end result is that it makes perhaps 1/3 as many errors as
the default Git algorithm, but that still leaves a lot of ugly diffs.

This commit implements a new and much better heuristic for picking
optimal "slider" positions using the following approach: First observe
that each hypothetical positioning of a diff slider introduces two
splits: one between the context lines preceding the group and the first
added/deleted line, and the other between the last added/deleted line
and the first line of context following it. It tries to find the
positioning that creates the least bad splits.

Splits are evaluated based only on the presence and locations of nearby
blank lines, and the indentation of lines near the split. Basically, it
prefers to introduce splits adjacent to blank lines, between lines that
are indented less, and between lines with the same level of indentation.
In more detail:

1. It measures the following characteristics of a proposed splitting
   position in a `struct split_measurement`:

   * the number of blank lines above the proposed split
   * whether the line directly after the split is blank
   * the number of blank lines following that line
   * the indentation of the nearest non-blank line above the split
   * the indentation of the line directly below the split
   * the indentation of the nearest non-blank line after that line

2. It combines the measured attributes using a bunch of
   empirically-optimized weighting factors to derive a `struct
   split_score` that measures the "badness" of splitting the text at
   that position.

3. It combines the `split_score` for the top and the bottom of the
   slider at each of its possible positions, and selects the position
   that has the best `split_score`.

I determined the initial set of weighting factors by collecting a corpus
of Git histories from 29 open-source software projects in various
programming languages. I generated many diffs from this corpus, and
determined the best positioning "by eye" for about 6600 diff sliders. I
used about half of the repositories in the corpus (corresponding to
about 2/3 of the sliders) as a training set, and optimized the weights
against this corpus using a crude automated search of the parameter
space to get the best agreement with the manually-determined values.
Then I tested the resulting heuristic against the full corpus. The
results are summarized in the following table, in column `indent-1`:

| repository            | count |      Git 2.9.0 |     compaction | compaction-fixed |       indent-1 |       indent-2 |
| --------------------- | ----- | -------------- | -------------- | ---------------- | -------------- | -------------- |
| afnetworking          |   109 |    89  (81.7%) |    37  (33.9%) |      37  (33.9%) |     2   (1.8%) |     2   (1.8%) |
| alamofire             |    30 |    18  (60.0%) |    14  (46.7%) |      15  (50.0%) |     0   (0.0%) |     0   (0.0%) |
| angular               |   184 |   127  (69.0%) |    39  (21.2%) |      23  (12.5%) |     5   (2.7%) |     5   (2.7%) |
| animate               |   313 |     2   (0.6%) |     2   (0.6%) |       2   (0.6%) |     2   (0.6%) |     2   (0.6%) |
| ant                   |   380 |   356  (93.7%) |   152  (40.0%) |     148  (38.9%) |    15   (3.9%) |    15   (3.9%) | *
| bugzilla              |   306 |   263  (85.9%) |   109  (35.6%) |      99  (32.4%) |    14   (4.6%) |    15   (4.9%) | *
| corefx                |   126 |    91  (72.2%) |    22  (17.5%) |      21  (16.7%) |     6   (4.8%) |     6   (4.8%) |
| couchdb               |    78 |    44  (56.4%) |    26  (33.3%) |      28  (35.9%) |     6   (7.7%) |     6   (7.7%) | *
| cpython               |   937 |   158  (16.9%) |    50   (5.3%) |      49   (5.2%) |     5   (0.5%) |     5   (0.5%) | *
| discourse             |   160 |    95  (59.4%) |    42  (26.2%) |      36  (22.5%) |    18  (11.2%) |    13   (8.1%) |
| docker                |   307 |   194  (63.2%) |   198  (64.5%) |     253  (82.4%) |     8   (2.6%) |     8   (2.6%) | *
| electron              |   163 |   132  (81.0%) |    38  (23.3%) |      39  (23.9%) |     6   (3.7%) |     6   (3.7%) |
| git                   |   536 |   470  (87.7%) |    73  (13.6%) |      78  (14.6%) |    16   (3.0%) |    16   (3.0%) | *
| gitflow               |   127 |     0   (0.0%) |     0   (0.0%) |       0   (0.0%) |     0   (0.0%) |     0   (0.0%) |
| ionic                 |   133 |    89  (66.9%) |    29  (21.8%) |      38  (28.6%) |     1   (0.8%) |     1   (0.8%) |
| ipython               |   482 |   362  (75.1%) |   167  (34.6%) |     169  (35.1%) |    11   (2.3%) |    11   (2.3%) | *
| junit                 |   161 |   147  (91.3%) |    67  (41.6%) |      66  (41.0%) |     1   (0.6%) |     1   (0.6%) | *
| lighttable            |    15 |     5  (33.3%) |     0   (0.0%) |       2  (13.3%) |     0   (0.0%) |     0   (0.0%) |
| magit                 |    88 |    75  (85.2%) |    11  (12.5%) |       9  (10.2%) |     1   (1.1%) |     0   (0.0%) |
| neural-style          |    28 |     0   (0.0%) |     0   (0.0%) |       0   (0.0%) |     0   (0.0%) |     0   (0.0%) |
| nodejs                |   781 |   649  (83.1%) |   118  (15.1%) |     111  (14.2%) |     4   (0.5%) |     5   (0.6%) | *
| phpmyadmin            |   491 |   481  (98.0%) |    75  (15.3%) |      48   (9.8%) |     2   (0.4%) |     2   (0.4%) | *
| react-native          |   168 |   130  (77.4%) |    79  (47.0%) |      81  (48.2%) |     0   (0.0%) |     0   (0.0%) |
| rust                  |   171 |   128  (74.9%) |    30  (17.5%) |      27  (15.8%) |    16   (9.4%) |    14   (8.2%) |
| spark                 |   186 |   149  (80.1%) |    52  (28.0%) |      52  (28.0%) |     2   (1.1%) |     2   (1.1%) |
| tensorflow            |   115 |    66  (57.4%) |    48  (41.7%) |      48  (41.7%) |     5   (4.3%) |     5   (4.3%) |
| test-more             |    19 |    15  (78.9%) |     2  (10.5%) |       2  (10.5%) |     1   (5.3%) |     1   (5.3%) | *
| test-unit             |    51 |    34  (66.7%) |    14  (27.5%) |       8  (15.7%) |     2   (3.9%) |     2   (3.9%) | *
| xmonad                |    23 |    22  (95.7%) |     2   (8.7%) |       2   (8.7%) |     1   (4.3%) |     1   (4.3%) | *
| --------------------- | ----- | -------------- | -------------- | ---------------- | -------------- | -------------- |
| totals                |  6668 |  4391  (65.9%) |  1496  (22.4%) |    1491  (22.4%) |   150   (2.2%) |   144   (2.2%) |
| totals (training set) |  4552 |  3195  (70.2%) |  1053  (23.1%) |    1061  (23.3%) |    86   (1.9%) |    88   (1.9%) |
| totals (test set)     |  2116 |  1196  (56.5%) |   443  (20.9%) |     430  (20.3%) |    64   (3.0%) |    56   (2.6%) |

In this table, the numbers are the count and percentage of human-rated
sliders that the corresponding algorithm got *wrong*. The columns are

* "repository" - the name of the repository used. I used the diffs
  between successive non-merge commits on the HEAD branch of the
  corresponding repository.

* "count" - the number of sliders that were human-rated. I chose most,
  but not all, sliders to rate from those among which the various
  algorithms gave different answers.

* "Git 2.9.0" - the default algorithm used by `git diff` in Git 2.9.0.

* "compaction" - the heuristic used by `git diff --compaction-heuristic`
  in Git 2.9.0.

* "compaction-fixed" - the heuristic used by `git diff
  --compaction-heuristic` after the fixes from earlier in this patch
  series. Note that the results are not dramatically different than
  those for "compaction". Both produce non-ideal diffs only about 1/3 as
  often as the default `git diff`.

* "indent-1" - the new `--indent-heuristic` algorithm, using the first
  set of weighting factors, determined as described above.

* "indent-2" - the new `--indent-heuristic` algorithm, using the final
  set of weighting factors, determined as described below.

* `*` - indicates that repo was part of training set used to determine
  the first set of weighting factors.

The fact that the heuristic performed nearly as well on the test set as
on the training set in column "indent-1" is a good indication that the
heuristic was not over-trained. Given that fact, I ran a second round of
optimization, using the entire corpus as the training set. The resulting
set of weights gave the results in column "indent-2". These are the
weights included in this patch.

The final result gives consistently and significantly better results
across the whole corpus than either `git diff` or `git diff
--compaction-heuristic`. It makes only about 1/30 as many errors as the
former and about 1/10 as many errors as the latter. (And a good fraction
of the remaining errors are for diffs that involve weirdly-formatted
code, sometimes apparently machine-generated.)

The tools that were used to do this optimization and analysis, along
with the human-generated data values, are recorded in a separate project
[1].

This patch adds a new command-line option `--indent-heuristic`, and a
new configuration setting `diff.indentHeuristic`, that activate this
heuristic. This interface is only meant for testing purposes, and should
be finalized before including this change in any release.

[1] https://github.com/mhagger/diff-slider-tools

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-19 10:25:11 -07:00
Jacob Keller fd47ae6a5b diff: teach diff to display submodule difference with an inline diff
Teach git-diff and friends a new format for displaying the difference
of a submodule. The new format is an inline diff of the contents of the
submodule between the commit range of the update. This allows the user
to see the actual code change caused by a submodule update.

Add tests for the new format and option.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-31 18:07:10 -07:00
Jacob Keller 660e113ce1 graph: add support for --line-prefix on all graph-aware output
Add an extension to git-diff and git-log (and any other graph-aware
displayable output) such that "--line-prefix=<string>" will print the
additional line-prefix on every line of output.

To make this work, we have to fix a few bugs in the graph API that force
graph_show_commit_msg to be used only when you have a valid graph.
Additionally, we extend the default_diff_output_prefix handler to work
even when no graph is enabled.

This is somewhat of a hack on top of the graph API, but I think it
should be acceptable here.

This will be used by a future extension of submodule display which
displays the submodule diff as the actual diff between the pre and post
commit in the submodule project.

Add some tests for both git-log and git-diff to ensure that the prefix
is honored correctly.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-31 18:07:09 -07:00
Junio C Hamano c3531e0385 Merge branch 'jc/doc-diff-filter-exclude'
Belated doc update for a feature added in v1.8.5.

* jc/doc-diff-filter-exclude:
  diff: document diff-filter exclusion
2016-07-25 14:13:41 -07:00
Junio C Hamano 16726cfa0c diff: document diff-filter exclusion
In v1.8.5 days, 7f2ea5f0 (diff: allow lowercase letter to specify
what change class to exclude, 2013-07-17) taught the "--diff-filter"
mechanism to take lowercase letters as exclusion, but we forgot to
document it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-14 12:17:47 -07:00
Junio C Hamano e5f7675544 Merge branch 'jk/diff-compact-heuristic'
It turns out that the earlier effort to update the heuristics may
want to use a bit more time to mature.  Turn it off by default.

* jk/diff-compact-heuristic:
  diff: disable compaction heuristic for now
2016-06-10 15:26:06 -07:00
Junio C Hamano 5580b271af diff: disable compaction heuristic for now
http://lkml.kernel.org/g/20160610075043.GA13411@sigill.intra.peff.net
reports that a change to add a new "function" with common ending
with the existing one at the end of the file is shown like this:

    def foo
      do_foo_stuff()

   +  common_ending()
   +end
   +
   +def bar
   +  do_bar_stuff()
   +
      common_ending()
    end

when the new heuristic is in use.  In reality, the change is to add
the blank line before "def bar" and everything below, which is what
the code without the new heuristic shows.

Disable the heuristics by default, and resurrect the documentation
for the option and the configuration variables, while clearly
marking the feature as still experimental.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-10 13:45:23 -07:00
Junio C Hamano 1cca17dfff Documentation: fix linkgit references
There are a handful of incorrect "linkgit:<page>[<section>]"
instances in our documentation set.

 * Some have an extra colon after "linkgit:"; fix them by removing
   the extra colon;

 * Some refer to a page outside the Git suite, namely curl(1); fix
   them by using the `curl(1)` that already appears on the same page
   for the same purpose of referring the readers to its manual page.

 * Some spell the name of the page incorrectly, e.g. "rev-list" when
   they mean "git-rev-list"; fix them.

 * Some list the manual section incorrectly; fix them to make sure
   they match what is at the top of the target of the link.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-09 15:44:14 -07:00