1. 28 1月, 2019 3 次提交
  2. 14 11月, 2018 1 次提交
    • S
      diff: align move detection error handling with other options · d173e799
      Stefan Beller 提交于
      This changes the error handling for the options --color-moved-ws
      and --color-moved-ws to be like the rest of the options.
      
      Move the die() call out of parse_color_moved_ws into the parsing
      of command line options. As the function returns a bit field, change
      its signature to return an unsigned instead of an int; add a new bit
      to signal errors. Once the error is signaled, we discard the other
      bits, such that it doesn't matter if the error bit overlaps with any
      other bit.
      Signed-off-by: NStefan Beller <sbeller@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      d173e799
  3. 12 11月, 2018 2 次提交
  4. 22 9月, 2018 4 次提交
  5. 21 8月, 2018 1 次提交
  6. 14 8月, 2018 4 次提交
    • N
      diff.c: move read_index() code back to the caller · ff7fe37b
      Nguyễn Thái Ngọc Duy 提交于
      This code is only needed for diff-tree (since f0c6b2a2 ([PATCH]
      Optimize diff-tree -[CM] --stdin - 2005-05-27)). Let the caller do the
      preparation instead and avoid read_index() in diff.c code.
      
      read_index() should be avoided (in addition to the_index) because it
      uses get_index_file() underneath to get the path $GIT_DIR/index. This
      effectively pulls the_repository in and may become the only reason to
      pull a 'struct repository *' in diff.c. Let's keep the dependencies as
      few as possible and kick it back to diff-tree.c
      Signed-off-by: NNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      ff7fe37b
    • J
      range-diff: use dim/bold cues to improve dual color mode · a7be92ac
      Johannes Schindelin 提交于
      It *is* a confusing thing to look at a diff of diffs. All too easy is it
      to mix up whether the -/+ markers refer to the "inner" or the "outer"
      diff, i.e. whether a `+` indicates that a line was added by either the
      old or the new diff (or both), or whether the new diff does something
      different than the old diff.
      
      To make things easier to process for normal developers, we introduced
      the dual color mode which colors the lines according to the commit diff,
      i.e. lines that are added by a commit (whether old, new, or both) are
      colored in green. In non-dual color mode, the lines would be colored
      according to the outer diff: if the old commit added a line, it would be
      colored red (because that line addition is only present in the first
      commit range that was specified on the command-line, i.e. the "old"
      commit, but not in the second commit range, i.e. the "new" commit).
      
      However, this dual color mode is still not making things clear enough,
      as we are looking at two levels of diffs, and we still only pick a color
      according to *one* of them (the outer diff marker is colored
      differently, of course, but in particular with deep indentation, it is
      easy to lose track of that outer diff marker's background color).
      
      Therefore, let's add another dimension to the mix. Still use
      green/red/normal according to the commit diffs, but now also dim the
      lines that were only in the old commit, and use bold face for the lines
      that are only in the new commit.
      
      That way, it is much easier not to lose track of, say, when we are
      looking at a line that was added in the previous iteration of a patch
      series but the new iteration adds a slightly different version: the
      obsolete change will be dimmed, the current version of the patch will be
      bold.
      
      At least this developer has a much easier time reading the range-diffs
      that way.
      Signed-off-by: NJohannes Schindelin <johannes.schindelin@gmx.de>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      a7be92ac
    • J
      diff: add an internal option to dual-color diffs of diffs · f7c3b4e2
      Johannes Schindelin 提交于
      When diffing diffs, it can be quite daunting to figure out what the heck
      is going on, as there are nested +/- signs.
      
      Let's make this easier by adding a flag in diff_options that allows
      color-coding the outer diff sign with inverted colors, so that the
      preimage and postimage is colored like the diff it is.
      
      Of course, this really only makes sense when the preimage and postimage
      *are* diffs. So let's not expose this flag via a command-line option for
      now.
      
      This is a feature that was invented by git-tbdiff, and it will be used
      by `git range-diff` in the next commit, by offering it via a new option:
      `--dual-color`.
      Signed-off-by: NJohannes Schindelin <johannes.schindelin@gmx.de>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      f7c3b4e2
    • J
      range-diff: suppress the diff headers · 1cdde296
      Johannes Schindelin 提交于
      When showing the diff between corresponding patches of the two branch
      versions, we have to make up a fake filename to run the diff machinery.
      
      That filename does not carry any meaningful information, hence tbdiff
      suppresses it. So we should, too.
      Signed-off-by: NJohannes Schindelin <johannes.schindelin@gmx.de>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      1cdde296
  7. 04 8月, 2018 1 次提交
  8. 20 7月, 2018 1 次提交
    • S
      diff.c: add white space mode to move detection that allows indent changes · ca1f4ae4
      Stefan Beller 提交于
      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
      2e2d5ac1 (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 2e2d5ac1 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: NStefan Beller <sbeller@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      ca1f4ae4
  9. 18 7月, 2018 2 次提交
    • S
      diff.c: decouple white space treatment from move detection algorithm · b3095712
      Stefan Beller 提交于
      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: NStefan Beller <sbeller@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      b3095712
    • S
      diff.c: add a blocks mode for moved code detection · 51da15eb
      Stefan Beller 提交于
      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: NÆvar Arnfjörð Bjarmason <avarab@gmail.com>
       (https://public-inbox.org/git/87o9j0uljo.fsf@evledraar.gmail.com/)
      Signed-off-by: NStefan Beller <sbeller@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      51da15eb
  10. 13 5月, 2018 1 次提交
    • B
      add status config and command line options for rename detection · e8b2dc2c
      Ben Peart 提交于
      After performing a merge that has conflicts git status will, by default,
      attempt to detect renames which causes many objects to be examined.  In a
      virtualized repo, those objects do not exist locally so the rename logic
      triggers them to be fetched from the server. This results in the status call
      taking hours to complete on very large repos vs seconds with this patch.
      
      Add a new config status.renames setting to enable turning off rename
      detection during status and commit.  This setting will default to the value
      of diff.renames.
      
      Add a new config status.renamelimit setting to to enable bounding the time
      spent finding out inexact renames during status and commit.  This setting
      will default to the value of diff.renamelimit.
      
      Add --no-renames command line option to status that enables overriding the
      config setting from the command line. Add --find-renames[=<n>] command line
      option to status that enables detecting renames and optionally setting the
      similarity index.
      Reviewed-by: NElijah Newren <newren@gmail.com>
      Original-Patch-by: NAlejandro Pauly <alpauly@microsoft.com>
      Signed-off-by: NBen Peart <Ben.Peart@microsoft.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      e8b2dc2c
  11. 08 5月, 2018 1 次提交
  12. 28 2月, 2018 1 次提交
    • N
      diff: add --compact-summary · ddf88fa6
      Nguyễn Thái Ngọc Duy 提交于
      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 0433d533:
      
         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: NNguyễn Thái Ngọc Duy <pclouds@gmail.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      ddf88fa6
  13. 05 1月, 2018 4 次提交
  14. 28 11月, 2017 1 次提交
  15. 01 11月, 2017 7 次提交
    • B
      diff: make struct diff_flags members lowercase · 0d1e0e78
      Brandon Williams 提交于
      Now that the flags stored in struct diff_flags are being accessed
      directly and not through macros, change all struct members from being
      uppercase to lowercase.
      This conversion is done using the following semantic patch:
      
      	@@
      	expression E;
      	@@
      	- E.RECURSIVE
      	+ E.recursive
      
      	@@
      	expression E;
      	@@
      	- E.TREE_IN_RECURSIVE
      	+ E.tree_in_recursive
      
      	@@
      	expression E;
      	@@
      	- E.BINARY
      	+ E.binary
      
      	@@
      	expression E;
      	@@
      	- E.TEXT
      	+ E.text
      
      	@@
      	expression E;
      	@@
      	- E.FULL_INDEX
      	+ E.full_index
      
      	@@
      	expression E;
      	@@
      	- E.SILENT_ON_REMOVE
      	+ E.silent_on_remove
      
      	@@
      	expression E;
      	@@
      	- E.FIND_COPIES_HARDER
      	+ E.find_copies_harder
      
      	@@
      	expression E;
      	@@
      	- E.FOLLOW_RENAMES
      	+ E.follow_renames
      
      	@@
      	expression E;
      	@@
      	- E.RENAME_EMPTY
      	+ E.rename_empty
      
      	@@
      	expression E;
      	@@
      	- E.HAS_CHANGES
      	+ E.has_changes
      
      	@@
      	expression E;
      	@@
      	- E.QUICK
      	+ E.quick
      
      	@@
      	expression E;
      	@@
      	- E.NO_INDEX
      	+ E.no_index
      
      	@@
      	expression E;
      	@@
      	- E.ALLOW_EXTERNAL
      	+ E.allow_external
      
      	@@
      	expression E;
      	@@
      	- E.EXIT_WITH_STATUS
      	+ E.exit_with_status
      
      	@@
      	expression E;
      	@@
      	- E.REVERSE_DIFF
      	+ E.reverse_diff
      
      	@@
      	expression E;
      	@@
      	- E.CHECK_FAILED
      	+ E.check_failed
      
      	@@
      	expression E;
      	@@
      	- E.RELATIVE_NAME
      	+ E.relative_name
      
      	@@
      	expression E;
      	@@
      	- E.IGNORE_SUBMODULES
      	+ E.ignore_submodules
      
      	@@
      	expression E;
      	@@
      	- E.DIRSTAT_CUMULATIVE
      	+ E.dirstat_cumulative
      
      	@@
      	expression E;
      	@@
      	- E.DIRSTAT_BY_FILE
      	+ E.dirstat_by_file
      
      	@@
      	expression E;
      	@@
      	- E.ALLOW_TEXTCONV
      	+ E.allow_textconv
      
      	@@
      	expression E;
      	@@
      	- E.TEXTCONV_SET_VIA_CMDLINE
      	+ E.textconv_set_via_cmdline
      
      	@@
      	expression E;
      	@@
      	- E.DIFF_FROM_CONTENTS
      	+ E.diff_from_contents
      
      	@@
      	expression E;
      	@@
      	- E.DIRTY_SUBMODULES
      	+ E.dirty_submodules
      
      	@@
      	expression E;
      	@@
      	- E.IGNORE_UNTRACKED_IN_SUBMODULES
      	+ E.ignore_untracked_in_submodules
      
      	@@
      	expression E;
      	@@
      	- E.IGNORE_DIRTY_SUBMODULES
      	+ E.ignore_dirty_submodules
      
      	@@
      	expression E;
      	@@
      	- E.OVERRIDE_SUBMODULE_CONFIG
      	+ E.override_submodule_config
      
      	@@
      	expression E;
      	@@
      	- E.DIRSTAT_BY_LINE
      	+ E.dirstat_by_line
      
      	@@
      	expression E;
      	@@
      	- E.FUNCCONTEXT
      	+ E.funccontext
      
      	@@
      	expression E;
      	@@
      	- E.PICKAXE_IGNORE_CASE
      	+ E.pickaxe_ignore_case
      
      	@@
      	expression E;
      	@@
      	- E.DEFAULT_FOLLOW_RENAMES
      	+ E.default_follow_renames
      Signed-off-by: NBrandon Williams <bmwill@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      0d1e0e78
    • B
      diff: remove DIFF_OPT_CLR macro · b2100e52
      Brandon Williams 提交于
      Remove the `DIFF_OPT_CLR` macro and instead set the flags directly.
      This conversion is done using the following semantic patch:
      
      	@@
      	expression E;
      	identifier fld;
      	@@
      	- DIFF_OPT_CLR(&E, fld)
      	+ E.flags.fld = 0
      
      	@@
      	type T;
      	T *ptr;
      	identifier fld;
      	@@
      	- DIFF_OPT_CLR(ptr, fld)
      	+ ptr->flags.fld = 0
      Signed-off-by: NBrandon Williams <bmwill@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      b2100e52
    • B
      diff: remove DIFF_OPT_SET macro · 23dcf77f
      Brandon Williams 提交于
      Remove the `DIFF_OPT_SET` macro and instead set the flags directly.
      This conversion is done using the following semantic patch:
      
      	@@
      	expression E;
      	identifier fld;
      	@@
      	- DIFF_OPT_SET(&E, fld)
      	+ E.flags.fld = 1
      
      	@@
      	type T;
      	T *ptr;
      	identifier fld;
      	@@
      	- DIFF_OPT_SET(ptr, fld)
      	+ ptr->flags.fld = 1
      Signed-off-by: NBrandon Williams <bmwill@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      23dcf77f
    • B
      diff: remove DIFF_OPT_TST macro · 3b69daed
      Brandon Williams 提交于
      Remove the `DIFF_OPT_TST` macro and instead access the flags directly.
      This conversion is done using the following semantic patch:
      
      	@@
      	expression E;
      	identifier fld;
      	@@
      	- DIFF_OPT_TST(&E, fld)
      	+ E.flags.fld
      
      	@@
      	type T;
      	T *ptr;
      	identifier fld;
      	@@
      	- DIFF_OPT_TST(ptr, fld)
      	+ ptr->flags.fld
      Signed-off-by: NBrandon Williams <bmwill@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      3b69daed
    • B
      diff: remove touched flags · 25567af8
      Brandon Williams 提交于
      Now that the set of parallel touched flags are no longer being used,
      remove them.
      Signed-off-by: NBrandon Williams <bmwill@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      25567af8
    • B
      diff: add flag to indicate textconv was set via cmdline · afa73c53
      Brandon Williams 提交于
      git-show is unique in that it wants to use textconv by default except
      for when it is showing blobs.  When asked to show a blob, show doesn't
      want to use textconv unless the user explicitly requested that it be
      used by providing the command line flag '--textconv'.
      
      Currently this is done by using a parallel set of 'touched' flags which
      get set every time a particular flag is set or cleared.  In a future
      patch we want to eliminate this parallel set of flags so instead of
      relying on if the textconv flag has been touched, add a new flag
      'TEXTCONV_SET_VIA_CMDLINE' which is only set if textconv is set to true
      via the command line.
      Signed-off-by: NBrandon Williams <bmwill@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      afa73c53
    • B
      diff: convert flags to be stored in bitfields · 02f2f56b
      Brandon Williams 提交于
      We cannot add many more flags to the diff machinery due to the
      limitations of the number of flags that can be stored in a single
      unsigned int.  In order to allow for more flags to be added to the diff
      machinery in the future this patch converts the flags to be stored in
      bitfields in 'struct diff_flags'.
      Signed-off-by: NBrandon Williams <bmwill@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      02f2f56b
  16. 14 10月, 2017 1 次提交
    • J
      revision: quit pruning diff more quickly when possible · a937b37e
      Jeff King 提交于
      When the revision traversal machinery is given a pathspec,
      we must compute the parent-diff for each commit to determine
      which ones are TREESAME. We set the QUICK diff flag to avoid
      looking at more entries than we need; we really just care
      whether there are any changes at all.
      
      But there is one case where we want to know a bit more: if
      --remove-empty is set, we care about finding cases where the
      change consists only of added entries (in which case we may
      prune the parent in try_to_simplify_commit()). To cover that
      case, our file_add_remove() callback does not quit the diff
      upon seeing an added entry; it keeps looking for other types
      of entries.
      
      But this means when --remove-empty is not set (and it is not
      by default), we compute more of the diff than is necessary.
      You can see this in a pathological case where a commit adds
      a very large number of entries, and we limit based on a
      broad pathspec. E.g.:
      
        perl -e '
          chomp(my $blob = `git hash-object -w --stdin </dev/null`);
          for my $a (1..1000) {
            for my $b (1..1000) {
              print "100644 $blob\t$a/$b\n";
            }
          }
        ' | git update-index --index-info
        git commit -qm add
      
        git rev-list HEAD -- .
      
      This case takes about 100ms now, but after this patch only
      needs 6ms. That's not a huge improvement, but it's easy to
      get and it protects us against even more pathological cases
      (e.g., going from 1 million to 10 million files would take
      ten times as long with the current code, but not increase at
      all after this patch).
      
      This is reported to minorly speed-up pathspec limiting in
      real world repositories (like the 100-million-file Windows
      repository), but probably won't make a noticeable difference
      outside of pathological setups.
      
      This patch actually covers the case without --remove-empty,
      and the case where we see only deletions. See the in-code
      comment for details.
      
      Note that we have to add a new member to the diff_options
      struct so that our callback can see the value of
      revs->remove_empty_trees. This callback parameter could be
      passed to the "add_remove" and "change" callbacks, but
      there's not much point. They already receive the
      diff_options struct, and doing it this way avoids having to
      update the function signature of the other callbacks
      (arguably the format_callback and output_prefix functions
      could benefit from the same simplification).
      Signed-off-by: NJeff King <peff@peff.net>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      a937b37e
  17. 17 8月, 2017 1 次提交
    • J
      diff: define block by number of alphanumeric chars · f0b8fb6e
      Jonathan Tan 提交于
      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: NJonathan Tan <jonathantanmy@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      f0b8fb6e
  18. 01 7月, 2017 4 次提交
    • S
      diff.c: add dimming to moved line detection · 86b452e2
      Stefan Beller 提交于
      Any lines inside a moved block of code are not interesting. Boundaries
      of blocks are only interesting if they are next to another block of moved
      code.
      Signed-off-by: NStefan Beller <sbeller@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      86b452e2
    • S
      diff.c: color moved lines differently, plain mode · 176841f0
      Stefan Beller 提交于
      Add the 'plain' mode for move detection of code. This omits the checking
      for adjacent blocks, so it is not as useful. If you have a lot of the
      same blocks moved in the same patch, the 'Zebra' would end up slow as it
      is O(n^2) (n is number of same blocks). So this may be useful there and
      is generally easy to add. Instead be very literal at the move detection,
      do not skip over short blocks here.
      Signed-off-by: NStefan Beller <sbeller@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      176841f0
    • S
      diff.c: color moved lines differently · 2e2d5ac1
      Stefan Beller 提交于
      When a patch consists mostly of moving blocks of code around, it can
      be quite tedious to ensure that the blocks are moved verbatim, and not
      undesirably modified in the move. To that end, color blocks that are
      moved within the same patch differently. For example (OM, del, add,
      and NM are different colors):
      
          [OM]  -void sensitive_stuff(void)
          [OM]  -{
          [OM]  -        if (!is_authorized_user())
          [OM]  -                die("unauthorized");
          [OM]  -        sensitive_stuff(spanning,
          [OM]  -                        multiple,
          [OM]  -                        lines);
          [OM]  -}
      
                 void another_function()
                 {
          [del] -        printf("foo");
          [add] +        printf("bar");
                 }
      
          [NM]  +void sensitive_stuff(void)
          [NM]  +{
          [NM]  +        if (!is_authorized_user())
          [NM]  +                die("unauthorized");
          [NM]  +        sensitive_stuff(spanning,
          [NM]  +                        multiple,
          [NM]  +                        lines);
          [NM]  +}
      
      However adjacent blocks may be problematic. For example, in this
      potentially malicious patch, the swapping of blocks can be spotted:
      
          [OM]  -void sensitive_stuff(void)
          [OM]  -{
          [OMA] -        if (!is_authorized_user())
          [OMA] -                die("unauthorized");
          [OM]  -        sensitive_stuff(spanning,
          [OM]  -                        multiple,
          [OM]  -                        lines);
          [OMA] -}
      
                 void another_function()
                 {
          [del] -        printf("foo");
          [add] +        printf("bar");
                 }
      
          [NM]  +void sensitive_stuff(void)
          [NM]  +{
          [NMA] +        sensitive_stuff(spanning,
          [NMA] +                        multiple,
          [NMA] +                        lines);
          [NM]  +        if (!is_authorized_user())
          [NM]  +                die("unauthorized");
          [NMA] +}
      
      If the moved code is larger, it is easier to hide some permutation in the
      code, which is why some alternative coloring is needed.
      
      This patch implements the first mode:
      * basic alternating 'Zebra' mode
        This conveys all information needed to the user.  Defer customization to
        later patches.
      
      First I implemented an alternative design, which would try to fingerprint
      a line by its neighbors to detect if we are in a block or at the boundary.
      This idea iss error prone as it inspected each line and its neighboring
      lines to determine if the line was (a) moved and (b) if was deep inside
      a hunk by having matching neighboring lines. This is unreliable as the
      we can construct hunks which have equal neighbors that just exceed the
      number of lines inspected. (Think of 'AXYZBXYZCXYZD..' with each letter
      as a line, that is permutated to AXYZCXYZBXYZD..').
      
      Instead this provides a dynamic programming greedy algorithm that finds
      the largest moved hunk and then has several modes on highlighting bounds.
      
      A note on the options '--submodule=diff' and '--color-words/--word-diff':
      In the conversion to use emit_line in the prior patches both submodules
      as well as word diff output carefully chose to call emit_line with sign=0.
      All output with sign=0 is ignored for move detection purposes in this
      patch, such that no weird looking output will be generated for these
      cases. This leads to another thought: We could pass on '--color-moved' to
      submodules such that they color up moved lines for themselves. If we'd do
      so only line moves within a repository boundary are marked up.
      
      It is useful to have moved lines colored, but there are annoying corner
      cases, such as a single line moved, that is very common. For example
      in a typical patch of C code, we have closing braces that end statement
      blocks or functions.
      
      While it is technically true that these lines are moved as they show up
      elsewhere, it is harmful for the review as the reviewers attention is
      drawn to such a minor side annoyance.
      
      For now let's have a simple solution of hardcoding the number of
      moved lines to be at least 3 before coloring them. Note, that the
      length is applied across all blocks to find the 'lonely' blocks
      that pollute new code, but do not interfere with a permutated
      block where each permutation has less lines than 3.
      Helped-by: NJonathan Tan <jonathantanmy@google.com>
      Signed-off-by: NStefan Beller <sbeller@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      2e2d5ac1
    • S
      diff.c: buffer all output if asked to · e6e045f8
      Stefan Beller 提交于
      Introduce a new option 'emitted_symbols' in the struct diff_options which
      controls whether all output is buffered up until all output is available.
      It is set internally in diff.c when necessary.
      
      We'll have a new struct 'emitted_string' in diff.c which will be used to
      buffer each line.  The emitted_string will duplicate the memory of the
      line to buffer as that is easiest to reason about for now. In a future
      patch we may want to decrease the memory usage by not duplicating all
      output for buffering but rather we may want to store offsets into the
      file or in case of hunk descriptions such as the similarity score, we
      could just store the relevant number and reproduce the text later on.
      
      This approach was chosen as a first step because it is quite simple
      compared to the alternative with less memory footprint.
      
      emit_diff_symbol factors out the emission part and depending on the
      diff_options->emitted_symbols the emission will be performed directly
      when calling emit_diff_symbol or after the whole process is done, i.e.
      by buffering we have add the possibility for a second pass over the
      whole output before doing the actual output.
      
      In 6440d341 (2012-03-14, diff: tweak a _copy_ of diff_options with
      word-diff) we introduced a duplicate diff options struct for word
      emissions as we may have different regex settings in there.
      When buffering the output, we need to operate on just one buffer,
      so we have to copy back the emissions of the word buffer into the
      main buffer.
      
      Unconditionally enable output via buffer in this patch as it yields
      a great opportunity for testing, i.e. all the diff tests from the
      test suite pass without having reordering issues (i.e. only parts
      of the output got buffered, and we forgot to buffer other parts).
      The test suite passes, which gives confidence that we converted all
      functions to use emit_string for output.
      Signed-off-by: NStefan Beller <sbeller@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      e6e045f8