Because of the way "--follow" is implemented, we must have
exactly one pathspec. "git log" enforces this restriction,
but other users of the revision traversal code do not. For
example, "git format-patch --follow" will segfault during
try_to_follow_renames, as we have no pathspecs at all.
We can push this check down into diff_setup_done, which is
probably a better place anyway. It is the diff code that
introduces this restriction, so other parts of the code
should not need to care themselves.
Reported-by: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Make "git grep" and "git show" pay attention to --textconv when
dealing with blob objects.
* mg/more-textconv:
grep: honor --textconv for the case rev:path
grep: allow to use textconv filters
t7008: demonstrate behavior of grep with textconv
cat-file: do not die on --textconv without textconv filters
show: honor --textconv for blobs
diff_opt: track whether flags have been set explicitly
t4030: demonstrate behavior of show with textconv
Teach "git diff --diff-filter" to express "I do not want to see
these classes of changes" more directly by listing only the
unwanted ones in lowercase (e.g. "--diff-filter=d" will show
everything but deletion) and deprecate "diff-files -q" which did
the same thing as "--diff-filter=d".
* jc/diff-filter-negation:
diff: deprecate -q option to diff-files
diff: allow lowercase letter to specify what change class to exclude
diff: reject unknown change class given to --diff-filter
diff: preparse --diff-filter string argument
diff: factor out match_filter()
diff: pass the whole diff_options to diffcore_apply_filter()
The condition in the ternary operator was wrong, hence the wrong char
pointer could be used as the parameter for show_submodule_summary.
one->path may be null, but we definitely need a non null path given
to the function.
Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
Acked-By: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The line being changed is deep inside the function builtin_diff.
The variable name_b, which is used to evaluate the ternary expression
must evaluate to true at that position, hence the replacement with
just name_b.
The name_b variable only occurs a few times in that lengthy function:
As a parameter to the function itself:
static void builtin_diff(const char *name_a,
const char *name_b,
...
The next occurrences are at:
/* Never use a non-valid filename anywhere if at all possible */
name_a = DIFF_FILE_VALID(one) ? name_a : name_b;
name_b = DIFF_FILE_VALID(two) ? name_b : name_a;
a_one = quote_two(a_prefix, name_a + (*name_a == '/'));
b_two = quote_two(b_prefix, name_b + (*name_b == '/'));
In the last line of this block 'name_b' is dereferenced and compared
to '/'. This would crash if name_b was NULL. Hence in the following code
we can assume name_b being non-null.
The next occurrence is just as a function argument, which doesn't change
the memory, which name_b points to, so the assumption name_b being not
null still holds:
emit_rewrite_diff(name_a, name_b, one, two,
textconv_one, textconv_two, o);
The next occurrence would be the line of this patch. As name_b still must
be not null, we can remove the ternary operator.
Inside the emit_rewrite_diff function there is a also a line
ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
which was also simplified as there is also a dereference before the
ternary operator.
Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Assorted code cleanups and a minor fix.
* sb/misc-fixes:
diff.c: Do not initialize a variable, which gets reassigned anyway.
commit: Fix a memory leak in determine_author_info
daemon.c:handle: Remove unneeded check for null pointer.
"git show -s" was less discoverable than it should be.
* mm/diff-no-patch-synonym-to-s:
Documentation/git-log.txt: capitalize section names
Documentation: move description of -s, --no-patch to diff-options.txt
Documentation/git-show.txt: include common diff options, like git-log.txt
diff: allow --patch & cie to override -s/--no-patch
diff: allow --no-patch as synonym for -s
t4000-diff-format.sh: modernize style
This reimplements the ancient "-q" option to "git diff-files" that
was inherited from "show-diff -q" in terms of "--diff-filter=d". We
will be deprecating the "-q" option, so let's issue a warning when
we do so.
Incidentally this also tentatively fixes "git diff --no-index" to
honor "-q" and hide deletions; the use will get the same warning.
We should remove the support for "-q" in a future version but it is
not that urgent.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
All options that trigger a patch output now override --no-patch.
The case of --binary deserves extra attention: the name may suggest that
it turns a normal patch into a binary patch, but it actually already
enables patch output when normally disabled (e.g. "git log --binary"
displays a patch), hence it makes sense for "git show --no-patch
--binary" to display the binary patch.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This follows the usual convention of having a --no-foo option to negate
--foo.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In order to express "we do not care about deletions", we had to say
"--diff-filter=ACMRTXUB", giving all the possible change class
except for the one we do not want, "D".
This is cumbersome. As all the change classes are in uppercase,
allow their lowercase counterpart to selectively exclude the class
from the output. When such a negated change class is in the input,
start the filter option with the full bits set.
This would allow us to express the old "show-diff -q" with
"git diff-files --diff-filter=d".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We used to accept "git diff --diff-filter=Q" (note that there is no
such change class 'Q') silently and showed no output (because there
is no such change class 'Q').
Error out when such an input is given.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of running strchr() on the list of status characters over
and over again, parse the --diff-filter option into bitfields and
use the bits to see if the change to the filepair matches the status
requested.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diffcore_apply_filter() checks if a filepair matches the filter
given with the "--diff-filter" option for each input filepairs with
a fairly complex expression in two places.
Create a helper function and call it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The --diff-filter=<arg> option given by the user is kept as a
string, and passed to the underlying diffcore_apply_filter()
function as a string for each resulting path we run number of
strchr() to see if each class of change among ACDMRTXUB is meant to
be given.
Change the function signature to pass the whole diff_options, so
that we can pre-parse this string in the next patch.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git diff" refused to even show difference when core.safecrlf is
set to true (i.e. error out) and there are offending lines in the
working tree files.
* jc/maint-diff-core-safecrlf:
diff: demote core.safecrlf=true to core.safecrlf=warn
I attempted to make index_state->cache[] a "const struct cache_entry **"
to find out how existing entries in index are modified and where. The
question I have is what do we do if we really need to keep track of on-disk
changes in the index. The result is
- diff-lib.c: setting CE_UPTODATE
- name-hash.c: setting CE_HASHED
- preload-index.c, read-cache.c, unpack-trees.c and
builtin/update-index: obvious
- entry.c: write_entry() may refresh the checked out entry via
fill_stat_cache_info(). This causes "non-const struct cache_entry
*" in builtin/apply.c, builtin/checkout-index.c and
builtin/checkout.c
- builtin/ls-files.c: --with-tree changes stagemask and may set
CE_UPDATE
Of these, write_entry() and its call sites are probably most
interesting because it modifies on-disk info. But this is stat info
and can be retrieved via refresh, at least for porcelain
commands. Other just uses ce_flags for local purposes.
So, keeping track of "dirty" entries is just a matter of setting a
flag in index modification functions exposed by read-cache.c. Except
unpack-trees, the rest of the code base does not do anything funny
behind read-cache's back.
The actual patch is less valueable than the summary above. But if
anyone wants to re-identify the above sites. Applying this patch, then
this:
diff --git a/cache.h b/cache.h
index 430d021..1692891 100644
--- a/cache.h
+++ b/cache.h
@@ -267,7 +267,7 @@ static inline unsigned int canon_mode(unsigned int mode)
#define cache_entry_size(len) (offsetof(struct cache_entry,name) + (len) + 1)
struct index_state {
- struct cache_entry **cache;
+ const struct cache_entry **cache;
unsigned int version;
unsigned int cache_nr, cache_alloc, cache_changed;
struct string_list *resolve_undo;
will help quickly identify them without bogus warnings.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Otherwise the user will not be able to start to guess where in the
contents in the working tree the offending unsafe CR lies.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The goal of the patch is to introduce the GNU diff
-B/--ignore-blank-lines as closely as possible. The short option is not
available because it's already used for "break-rewrites".
When this option is used, git-diff will not create hunks that simply
add or remove empty lines, but will still show empty lines
addition/suppression if they are close enough to "valuable" changes.
There are two differences between this option and GNU diff -B option:
- GNU diff doesn't have "--inter-hunk-context", so this must be handled
- The following sequence looks like a bug (context is displayed twice):
$ seq 5 >file1
$ cat <<EOF >file2
change
1
2
3
4
5
change
EOF
$ diff -u -B file1 file2
--- file1 2013-06-08 22:13:04.471517834 +0200
+++ file2 2013-06-08 22:13:23.275517855 +0200
@@ -1,5 +1,7 @@
+change
1
2
+
3
4
5
@@ -3,3 +5,4 @@
3
4
5
+change
So here is a more thorough description of the option:
- real changes are interesting
- blank lines that are close enough (less than context size) to
interesting changes are considered interesting (recursive definition)
- "context" lines are used around each hunk of interesting changes
- If two hunks are separated by less than "inter-hunk-context", they
will be merged into one.
The implementation does the "interesting changes selection" in a single
pass.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The diff_opt infrastructure sets flags based on defaults and command
line options. It is impossible to tell whether a flag has been set
as a default or on explicit request. Update the structure so that
this detection is possible:
* Add an extra "opt->touched_flags" that keeps track of all the
fields that have been touched by DIFF_OPT_SET and DIFF_OPT_CLR.
* You may continue setting the default values to the flags, like
commands in the "log" family do in cmd_log_init_defaults(), but
after you finished setting the defaults, you clear the
touched_flags field;
* And then you let the usual callchain call diff_opt_parse(),
allowing the opt->flags be set or unset, while keeping track of
which bits the user touched;
* There is an optional callback "opt->set_default" that is called
at the very beginning to let you inspect touched_flags and update
opt->flags appropriately, before the remainder of the diffcore
machinery is set up, taking the opt->flags value into account.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git diff --diff-algorithm=algo" was understood by the command line
parser, but "git diff --diff-algorithm algo" was not.
* jk/diff-algo-finishing-touches:
diff: allow unstuck arguments with --diff-algorithm
git-merge(1): document diff-algorithm option to merge-recursive
"git diff --diff-algorithm algo" is also understood as "git diff
--diff-algorithm=algo".
* jk/diff-algo-finishing-touches:
diff: allow unstuck arguments with --diff-algorithm
git-merge(1): document diff-algorithm option to merge-recursive
Most of these were found using Lucas De Marchi's codespell tool.
Signed-off-by: Stefano Lattarini <stefano.lattarini@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When running "git log -p --submodule=log", the submodule log is not
indented by the graph output, although all other lines are. Fix this by
prepending the current line prefix to each line of the submodule log.
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The argument to --diff-algorithm is mandatory, so there is no reason to
require the argument to be stuck to the option with '='. Change this
for consistency with other Git commands.
Note that this does not change the handling of diff-algorithm in
merge-recursive.c since the primary interface to that is via the -X
option to 'git merge' where the unstuck form does not make sense.
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* ap/maint-diff-rename-avoid-overlap:
tests: make sure rename pretty print works
diff: prevent pprint_rename from underrunning input
diff: Fix rename pretty-print when suffix and prefix overlap
The logic used by "git diff -M --stat" to shorten the names of
files before and after a rename did not work correctly when the
common prefix and suffix between the two filenames overlapped.
* ap/maint-diff-rename-avoid-overlap:
tests: make sure rename pretty print works
diff: prevent pprint_rename from underrunning input
diff: Fix rename pretty-print when suffix and prefix overlap
In the warning message printed when rename or unmodified copy
detection was skipped due to too many files, change "diff.renamelimit"
to "diff.renameLimit", in order to make it consistent with git
documentation, which consistently uses "diff.renameLimit".
Signed-off-by: Max Nanasy <max.nanasy@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The logic described in d020e27 (diff: Fix rename pretty-print when
suffix and prefix overlap, 2013-02-23) is wrong: The proof in the
comment is valid only if both strings are the same length. *One* of
old/new can reach a-1 (b-1, resp.) if 'a' is a suffix of 'b' (or vice
versa).
Since the intent was to let the loop run down to the '/' at the end of
the common prefix, fix it by making that distinction explicit: if
there is no prefix, allow no underrun.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When considering a rename for two files that have a suffix and a prefix
that can overlap, a confusing line is shown. As an example, renaming
"a/b/b/c" to "a/b/c" shows "a/b/{ => }/b/c".
Currently, what we do is calculate the common prefix ("a/b/"), and the
common suffix ("/b/c"), but the same "/b/" is actually counted both in
prefix and suffix. Then when calculating the size of the non-common part,
we end-up with a negative value which is reset to 0, thus the "{ => }".
Do not allow the common suffix to overlap the common prefix and stop
when reaching a "/" that would be in both.
Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add diff.algorithm configuration so that the user does not type
"diff --histogram".
* mp/diff-algo-config:
diff: Introduce --diff-algorithm command line option
config: Introduce diff.algorithm variable
git-completion.bash: Autocomplete --minimal and --histogram for git-diff
Refactors a lot of repetitive code sequence from the graph drawing
code and adds it to the combined diff output.
* jk/diff-graph-cleanup:
combine-diff.c: teach combined diffs about line prefix
diff.c: use diff_line_prefix() where applicable
diff: add diff_line_prefix function
diff.c: make constant string arguments const
diff: write prefix to the correct file
graph: output padding for merge subsequent parents
This is a helper function to call the diff output_prefix function and
return its value as a C string, allowing us to greatly simplify
everywhere that needs to get the output prefix.
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Write the prefix for an output line to the same file as the actual
content.
Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since command line options have higher priority than config file
variables and taking previous commit into account, we need a way
how to specify myers algorithm on command line. However,
inventing `--myers` is not the right answer. We need far more
general option, and that is `--diff-algorithm`.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some users or projects prefer different algorithms over others, e.g.
patience over myers or similar. However, specifying appropriate
argument every time diff is to be used is impractical. Moreover,
creating an alias doesn't play nicely with other tools based on diff
(git-show for instance). Hence, a configuration variable which is able
to set specific algorithm is needed. For now, these four values are
accepted: 'myers' (which has the same effect as not setting the config
variable at all), 'minimal', 'patience' and 'histogram'.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git diff --stat" miscounted the total number of changed lines when
binary files were involved and hidden beyond --stat-count. It also
miscounted the total number of changed files when there were
unmerged paths.
* lt/diff-stat-show-0-lines:
t4049: refocus tests
diff --shortstat: do not count "unmerged" entries
diff --stat: do not count "unmerged" entries
diff --stat: move the "total count" logic to the last loop
diff --stat: use "file" temporary variable to refer to data->files[i]
diff --stat: status of unmodified pair in diff-q is not zero
test: add failing tests for "diff --stat" to t4049
Even though we show a separate *UNMERGED* entry in the patch and
diffstat output (or in the --raw format, for that matter) in
addition to and separately from the diff against the specified stage
(defaulting to #2) for unmerged paths, they should not be counted in
the total number of files affected---that would lead to counting the
same path twice.
The separation done by the previous step makes this fix simple and
straightforward. Among the filepairs in diff_queue, paths that
weren't modified, and the extra "unmerged" entries do not count as
total number of files.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The diffstat generation logic, with --stat-count limit, is
implemented as three loops.
- The first counts the width necessary to show stats up to
specified number of entries, and notes up to how many entries in
the data we need to iterate to show the graph;
- The second iterates that many times to draw the graph, adjusts
the number of "total modified files", and counts the total
added/deleted lines for the part that was shown in the graph;
- The third iterates over the remainder and only does the part to
count "total added/deleted lines" and to adjust "total modified
files" without drawing anything.
Move the logic to count added/deleted lines and modified files from
the second loop to the third loop.
This incidentally fixes a bug. The third loop was not filtering
binary changes (counted in bytes) from the total added/deleted as it
should. The second loop implemented this correctly, so if a binary
change appeared earlier than the --stat-count cutoff, the code
counted number of added/deleted lines correctly, but if it appeared
beyond the cutoff, the number of lines would have mixed with the
byte count in the buggy third loop.
Signed-off-by: Junio C Hamano <gitster@pobox.com>