I forgot to run format-patch with the -M flag
cover I forgot to run format-patch with the -M flag
Now I can forget more often :) Unless the similarity is too low.
I'm not too sure if I'd want to set similarity or find-copies-harder into my config just yet, since they depend on the situation. But I usually want renames in my diffs.
Signature · 1 line
Eric Wong
PATCH 1/1 diff.c: respect diff.renames config option
diff.renames is mentioned several times in the documentation, but to my surprise it didn't do anything before this patch.
--- Documentation/config.txt | 5 +++++ diff.c | 12 ++++++++++++ 2 files changed, 17 insertions(+), 0 deletions(-)
Documentation/config.txt +5 -0
@@ -114,6 +114,11 @@ diff.renameLimit:: The number of files to consider when performing the copy/rename detection; equivalent to the git diff option '-l'. +diff.renames:: + Tells git to detect renames. If set to any boolean value, it + will enable basic rename detection. If set to "copies" or + "copy", it will detect copies, as well. + format.headers:: Additional email headers to include in a patch to be submitted by mail. See gitlink:git-format-patch[1].
diff.c +12 -0
@@ -13,6 +13,7 @@ #include "xdiff-interface.h"static int use_size_cache; +static int diff_detect_rename_default =0 ;static int diff_rename_limit_default =-1 ;static int diff_use_color_default =0 ;
@@ -120,6 +121,16 @@ int git_diff_config(const char *var, con diff_use_color_default =git_config_bool (var, value);return 0 ; } +if (!strcmp (var,"diff.renames" )) { +if (!value) + diff_detect_rename_default =DIFF_DETECT_RENAME ; +else if (!strcasecmp (value,"copies" ) || + !strcasecmp (value,"copy" )) + diff_detect_rename_default =DIFF_DETECT_COPY ; +else + diff_detect_rename_default =git_config_bool (var,value); +return 0 ; + }if (!strncmp (var,"diff.color." ,11 )) {int slot =parse_diff_color_slot (var,11 ); diff_colors[slot] =parse_diff_color_value (value, var);
@@ -1429,6 +1440,7 @@ void diff_setup(struct diff_options *opt options->change = diff_change; options->add_remove = diff_addremove; options->color_diff = diff_use_color_default; + options->detect_rename = diff_detect_rename_default; }int diff_setup_done (struct diff_options *options)
Signature · 1 line
1.4.1.g3dc65
PATCH 1/1 builtin-log: respect diff configuration options
The log commands are all capable of generating diffs, so we should respect those configuration options for diffs here.
--- builtin-log.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
builtin-log.c +5 -2
@@ -47,6 +47,7 @@ int cmd_whatchanged(int argc, const char {struct rev_info rev; +git_config (git_diff_config);init_revisions (&rev); rev.diff =1 ; rev.diffopt.recursive =1 ;
@@ -61,6 +62,7 @@ int cmd_show(int argc, const char **argv {struct rev_info rev; +git_config (git_diff_config);init_revisions (&rev); rev.diff =1 ; rev.diffopt.recursive =1 ;
@@ -77,6 +79,7 @@ int cmd_log(int argc, const char **argv, {struct rev_info rev; +git_config (git_diff_config);init_revisions (&rev); rev.always_show_header =1 ;cmd_log_init (argc, argv, envp, &rev);
@@ -102,7 +105,7 @@ static int git_format_config(const charstrcat (extra_headers, value);return 0 ; } -return git_default_config (var, value); +return git_diff_config (var, value); }
@@ -234,6 +237,7 @@ int cmd_format_patch(int argc, const chastruct diff_options patch_id_opts;char *add_signoff =NULL ; +git_config (git_format_config);init_revisions (&rev); rev.commit_format =CMIT_FMT_EMAIL ; rev.verbose_header =1 ;
@@ -243,7 +247,6 @@ int cmd_format_patch(int argc, const cha rev.diffopt.msg_sep ="" ; rev.diffopt.recursive =1 ; -git_config (git_format_config); rev.extra_headers = extra_headers;/*
Signature · 1 line
1.4.1.g3dc65
Re: [PATCH] builtin-log: respect diff configuration options
Quoted 4 lines · Eric Wong
The log commands are all capable of generating diffs, so we should respect those configuration options for diffs here.
Signed-off-by Eric Wong <[email protected]>
Makes sense. This makes "git log -p" more colorful ;-).
Re: [PATCH] diff.c: respect diff.renames config option
In reply to Eric WongQuoted 2 lines · Eric Wong
diff.renames is mentioned several times in the documentation, but to my surprise it didn't do anything before this patch.
I am really reluctant to do this at this low-level, especially since there is no way to defeat the settings from the command line.
Quoted 4 lines
diff.c +0
-0@@ -120,6 +121,16 @@ int git_diff_config(const char *var, con diff_use_color_default =git_config_bool (var, value);return 0 ; }
diff.c int git_diff_config(const char *var, con · line 133
Quoted 10 lines
diff.c +10
-0+if (!strcmp (var,"diff.renames" )) { +if (!value) + diff_detect_rename_default =DIFF_DETECT_RENAME ; +else if (!strcasecmp (value,"copies" ) || + !strcasecmp (value,"copy" )) + diff_detect_rename_default =DIFF_DETECT_COPY ; +else + diff_detect_rename_default =git_config_bool (var,value); +return 0 ; + }
I suspect this works only because DIFF_DETECT_RENAME happens to be 1?
A message that is not in the archive (sent off-list or not yet received): 20060707110123.GA23400@soma
Re: [PATCH] diff.c: respect diff.renames config option
Quoted 3 lines · Eric Wong
Nevertheless, it's still opt -in via repo-config, and most people will find renames useful unless they need to export to non-git systems.
I am more worried about somebody who opts-in finds breakage of commands that happen to internally use low-level diff machinery and expect the diff machinery does not automagically do funny rename detection without being told.
For example, revision walking with path pruning uses diff machinery without renames. I do not know what happens if I override it with diff.renames to allow rename detection but I fear something might horribly break.
That is why I said I do not want this at _that_ low level. I do not have objections to have the configuration at a layer closer to the UI, e.g. things in builtin-log.c and builtin-diff.c.
Re: [PATCH] diff.c: respect diff.renames config option
Quoted 8 lines · Junio C Hamano
I am more worried about somebody who opts-in finds breakage of commands that happen to internally use low-level diff machinery and expect the diff machinery does not automagically do funny rename detection without being told.
...
That is why I said I do not want this at _that_ low level. I do not have objections to have the configuration at a layer closer to the UI, e.g. things inbuiltin-log.candbuiltin-diff.c.
Upon closer look I think the revision pruning code is OK. So let's cook this as is in "next" and see what happens.
PATCH 1/1 Fix several places where diff.renames in config can be problematic
git-cvsexportcommit.perl:
git-cvsserver.perl:
CVS can't handle renames, so we best not show them to users.
templates/hooks--update:
replace diffstat calls with git diff --stat
There may be other places where users having diff.renames can be problematic, too. diff.renames is opt-in, but maybe some users have enabled it in their configs previously because it's been in examples for a long time...
---
Quoted 11 lines · Junio C Hamano
Quoted 8 lines · Junio C Hamano
I am more worried about somebody who opts-in finds breakage of commands that happen to internally use low-level diff machinery and expect the diff machinery does not automagically do funny rename detection without being told.
...
That is why I said I do not want this at _that_ low level. I do not have objections to have the configuration at a layer closer to the UI, e.g. things inbuiltin-log.candbuiltin-diff.c.
Quoted 2 lines
Upon closer look I think the revision pruning code is OK. So let's cook this as is in "next" and see what happens.
Cool. I've found these that could be potential issues. There could be more, and possibly many in 3rd-party scripts outside our control, too.
git-cvsexportcommit.perl | 8 +++++--- git-cvsserver.perl | 2 +- templates/hooks--update | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-)
git-cvsexportcommit.perl +5 -3
@@ -91,7 +91,8 @@ close MSG; $? && die "Error extracting the commit message"; my (@afiles, @dfiles, @mfiles, @dirs); -my @files = safe_pipe_capture('git-diff-tree', '-r', $parent, $commit); +my @files = safe_pipe_capture('git-diff-tree','--no-renames','-r', + $parent, $commit); #print @files; $? && die "Error in git-diff-tree"; foreach my $f (@files) {
@@ -175,7 +176,8 @@ foreach my $d (@dirs) { print "'Patching' binary files\n"; -my @bfiles = grep(m/^Binary/, safe_pipe_capture('git-diff-tree', '-p', $parent, $commit)); +my @bfiles = grep(m/^Binary/, safe_pipe_capture('git-diff-tree','--no-renames', + '-p', $parent, $commit)); @bfiles = map { chomp } @bfiles; foreach my $f (@bfiles) { # check that the file in cvs matches the "old" file
@@ -206,7 +208,7 @@ ## apply non-binary changes my $fuzz = $opt_p ? 0 : 2; print "Patching non-binary files\n"; -print `(git-diff-tree -p $parent -p $commit | patch -p1 -F $fuzz ) 2>&1`; +print `(git-diff-tree --no-renames -p $parent -p $commit | patch -p1 -F $fuzz ) 2>&1`; my $dirtypatch = 0; if (($? >> 8) == 2) {
git-cvsserver.perl +1 -1
@@ -2295,7 +2295,7 @@ sub update if ( defined ( $lastpicked ) ) { - my $filepipe = open(FILELIST, '-|', 'git-diff-tree', '-r', $lastpicked, $commit->{hash}) or die("Cannot call git-diff-tree : $!"); + my $filepipe = open(FILELIST, '-|', 'git-diff-tree', '--no-merges', '-r', $lastpicked, $commit->{hash}) or die("Cannot call git-diff-tree : $!"); while ( <FILELIST> ) { unless ( /^:\d{6}\s+\d{3}(\d)\d{2}\s+[a-zA-Z0-9]{40}\s+([a-zA-Z0-9]{40})\s+(\w)\s+(.*)$/o )
templates/hooks--update +2 -2
@@ -60,7 +60,7 @@ then echo "Changes since $prev:" git rev-list --pretty $prev..$3 | $short echo --- - git diff $prev..$3 | diffstat -p1 + git diff --stat $prev..$3 echo --- fi ;;
@@ -75,7 +75,7 @@ else base=$(git-merge-base "$2" "$3") case "$base" in "$2") - git diff "$3" "^$base" | diffstat -p1 + git diff --stat "$3" "^$base" echo echo "New commits:" ;;
Signature · 1 line
1.4.1.gc57e
Re: [PATCH] Fix several places where diff.renames in config can be problematic
Review
Quoted 3 lines · Eric Wong
(quoted diff) +2
-1-my @files = safe_pipe_capture('git-diff-tree', '-r', $parent, $commit); +my @files = safe_pipe_capture('git-diff-tree','--no-renames','-r', + $parent, $commit);
I changed my mind.
-- >8 --
diff: do not use configuration magic at the core-level
The Porcelainish has become so much usable as the UI that there is not much reason people should be using the core programs by hand anymore. At this point we are better off making the behaviour of the core programs predictable by keeping them unaffected by the configuration variables. Otherwise they will become very hard to use as reliable building blocks.
For example, "git-commit -a" internally uses git-diff-files to figure out the set of paths that need to be updated in the index, and we should never allow diff.renames that happens to be in the configuration to interfere (or slow down the process).
The UI level configuration such as showing renamed diff and coloring are still honored by the Porcelainish ("git log" family and "git diff"), but not by the core anymore.
--- builtin-diff-files.c | 2 +- builtin-diff-index.c | 2 +- builtin-diff-stages.c | 2 +- builtin-diff-tree.c | 2 +- builtin-diff.c | 2 +- builtin-log.c | 8 ++++---- diff.c | 8 +++++++- diff.h | 2 +- 8 files changed, 17 insertions(+), 11 deletions(-)
builtin-diff-files.c +1 -1
@@ -18,7 +18,7 @@ int cmd_diff_files(int argc, const charstruct rev_info rev;int silent =0 ; -git_config (git_diff_config); +git_config (git_default_config);/* no "diff" UI options */ init_revisions (&rev); rev.abbrev =0 ;
builtin-diff-index.c +1 -1
@@ -15,7 +15,7 @@ int cmd_diff_index(int argc, const charint cached =0 ;int i; -git_config (git_diff_config); +git_config (git_default_config);/* no "diff" UI options */ init_revisions (&rev); rev.abbrev =0 ;
builtin-diff-stages.c +1 -1
@@ -61,7 +61,7 @@ int cmd_diff_stages(int ac, const char *const char *prefix =setup_git_directory ();const char **pathspec =NULL ; -git_config (git_diff_config); +git_config (git_default_config);/* no "diff" UI options */ read_cache ();diff_setup (&diff_options);while (1 < ac && av[1 ][0 ] =='-' ) {
builtin-diff-tree.c +1 -1
@@ -67,7 +67,7 @@ int cmd_diff_tree(int argc, const char *static struct rev_info *opt = &log_tree_opt;int read_stdin =0 ; -git_config (git_diff_config); +git_config (git_default_config);/* no "diff" UI options */ nr_sha1 =0 ;init_revisions (opt); opt->abbrev =0 ;
builtin-diff.c +1 -1
@@ -250,7 +250,7 @@ int cmd_diff(int argc, const char **argv* Other cases are errors. */ -git_config (git_diff_config); +git_config (git_diff_ui_config);init_revisions (&rev); argc =setup_revisions (argc, argv, &rev,NULL );
builtin-log.c +4 -4
@@ -47,7 +47,7 @@ int cmd_whatchanged(int argc, const char {struct rev_info rev; -git_config (git_diff_config); +git_config (git_diff_ui_config);init_revisions (&rev); rev.diff =1 ; rev.diffopt.recursive =1 ;
@@ -62,7 +62,7 @@ int cmd_show(int argc, const char **argv {struct rev_info rev; -git_config (git_diff_config); +git_config (git_diff_ui_config);init_revisions (&rev); rev.diff =1 ; rev.diffopt.recursive =1 ;
@@ -79,7 +79,7 @@ int cmd_log(int argc, const char **argv, {struct rev_info rev; -git_config (git_diff_config); +git_config (git_diff_ui_config);init_revisions (&rev); rev.always_show_header =1 ;cmd_log_init (argc, argv, envp, &rev);
@@ -105,7 +105,7 @@ static int git_format_config(const charstrcat (extra_headers, value);return 0 ; } -return git_diff_config (var, value); +return git_diff_ui_config (var, value); }
diff.c +7 -1
@@ -102,7 +102,13 @@ static const char *parse_diff_color_valudie ("bad config value '%s' for variable '%s'" , value, var); } -int git_diff_config (const char *var,const char *value) +/* +* These are to give UI layer defaults. +* The core-level commands such as git-diff-files should +* never be affected by the setting of diff.renames +* the user happens to have in the configuration file. +*/ +int git_diff_ui_config (const char *var,const char *value) {if (!strcmp (var,"diff.renamelimit" )) { diff_rename_limit_default =git_config_int (var, value);
diff.h +1 -1
@@ -123,7 +123,7 @@ #define DIFF_SETUP_REVERSE 1#define DIFF_SETUP_USE_CACHE 2#define DIFF_SETUP_USE_SIZE_CACHE 4 -extern int git_diff_config (const char *var,const char *value); +extern int git_diff_ui_config (const char *var,const char *value);extern void diff_setup (struct diff_options *);extern int diff_opt_parse (struct diff_options *,const char **,int );extern int diff_setup_done (struct diff_options *);
Re: [PATCH] Fix several places where diff.renames in config can be problematic
Quoted 18 lines · Junio C Hamano
Review
Quoted 3 lines · Eric Wong
(quoted diff) +2
-1-my @files = safe_pipe_capture('git-diff-tree', '-r', $parent, $commit); +my @files = safe_pipe_capture('git-diff-tree','--no-renames','-r', + $parent, $commit);I changed my mind.
-- >8 --
diff: do not use configuration magic at the core-levelThe Porcelainish has become so much usable as the UI that there is not much reason people should be using the core programs by hand anymore. At this point we are better off making the behaviour of the core programs predictable by keeping them unaffected by the configuration variables. Otherwise they will become very hard to use as reliable building blocks.
Quoted 8 lines
For example, "git-commit -a" internally uses git-diff-files to figure out the set of paths that need to be updated in the index, and we should never allow diff.renames that happens to be in the configuration to interfere (or slow down the process).
The UI level configuration such as showing renamed diff and coloring are still honored by the Porcelainish ("git log" family and "git diff"), but not by the core anymore.
Full ack on this. I was ready to let my diff.renames patch drop if there were too many potential incompatibilities/breakages, but this should alleviate that.
I should work on breaking out of the habit of using git diff-{index,tree} in my day-to-day use and finally start using git diff more to save some keystrokes.
Signature · 1 line
Eric Wong
PATCH 1/1 templates/hooks--update: replace diffstat calls with git diff --stat
---
This was part of the previous patch, but useful on its own since we have our own internal diffstat
templates/hooks--update | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
templates/hooks--update +2 -2
@@ -60,7 +60,7 @@ then echo "Changes since $prev:" git rev-list --pretty $prev..$3 | $short echo --- - git diff $prev..$3 | diffstat -p1 + git diff --stat $prev..$3 echo --- fi ;;
@@ -75,7 +75,7 @@ else base=$(git-merge-base "$2" "$3") case "$base" in "$2") - git diff "$3" "^$base" | diffstat -p1 + git diff --stat "$3" "^$base" echo echo "New commits:" ;;
Signature · 1 line
1.4.1.g6a62
PATCH 1/1 update Documentation/diff-options.txt
In reply to Junio C HamanoWe've been lazy and left things undocumented for some time.
--- Documentation/diff-options.txt | 24 +++++++++++++++++++++--- 1 files changed, 21 insertions(+), 3 deletions(-)
Documentation/diff-options.txt +21 -3
@@ -4,18 +4,21 @@ -u:: Synonym for "-p". +--raw:: + Generate the raw format. + --patch-with-raw:: - Generate patch but keep also the default raw diff output. + Synonym for "-p --raw". --stat:: - Generate a diffstat instead of a patch. + Generate a diffstat. --summary:: Output a condensed summary of extended header information such as creations, renames and mode changes. --patch-with-stat:: - Generate patch and prepend its diffstat. + Synonym for "-p --stat". -z:: \0 line termination on output
@@ -26,11 +29,26 @@ --name-status:: Show only names and status of changed files. +--color:: + Show colored diff. + +--no-color:: + Turn off colored diff, even when the configuration file + gives the default to color output. + +--no-renames:: + Turn off rename detection, even when the configuration + file gives the default to do so. + --full-index:: Instead of the first handful characters, show full object name of pre- and post-image blob on the "index" line when generating a patch format output. +--binary:: + In addition to --full-index, output "binary diff" that + can be applied with "git apply". + --abbrev[=<n>]:: Instead of showing the full 40-byte hexadecimal object name in diff-raw format output and diff-tree header
Signature · 1 line
1.4.1.gfff4c
Re: [PATCH] diff.c: respect diff.renames config option
In reply to Junio C HamanoIn the same spirit...
From: Junio C Hamano <[email protected]>
Date: Fri, 7 Jul 2006 05:27:24 -0700
Subject: [PATCH] diff.c: --no-color to defeat diff.color configuration.
--- diff.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff.c +2 -0
@@ -1622,6 +1622,8 @@ int diff_opt_parse(struct diff_options * }else if (!strcmp (arg,"--color" )) options->color_diff =1 ; +else if (!strcmp (arg,"--no-color" )) + options->color_diff =0 ;else if (!strcmp (arg,"-w" ) || !strcmp (arg,"--ignore-all-space" )) options->xdl_opts |=XDF_IGNORE_WHITESPACE ;else if (!strcmp (arg,"-b" ) || !strcmp (arg,"--ignore-space-change" ))
Signature · 1 line
1.4.1.gfff4c