{"thread":{"id":"20930","subject":"rename tracking and file-name swapping","startedAt":"2009-09-13T11:17:13Z","lastAt":"2009-09-15T05:05:57Z","messageCount":7,"participants":["Yuri D'Elia","Junio C Hamano","Johannes Sixt","Nanako Shiraishi"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"123051","messageId":"wavexx-2ECE7F.13171313092009@ger.gmane.org","threadId":"20930","inReplyTo":null,"subject":"rename tracking and file-name swapping","fromName":"Yuri D'Elia","fromEmail":"wavexx@users.sf.net","sentAt":"2009-09-13T11:17:13Z","receivedAt":"2009-09-13T11:17:13Z","isPatch":false,"sender":{"key":"wavexx@users.sf.net","avatar":null},"body":"Hi everyone. Does rename tracking recognize two file names being swapped?\n\n% ls -l\ntotal 24\n-rw-rw-r-- 1 wavexx wavexx 5952 Sep 13 13:09 file1.txt\n-rw-rw-r-- 1 wavexx wavexx 3330 Sep 13 13:09 file2.txt\n% mv file1.txt file3.txt \n% mv file2.txt file1.txt\n% mv file3.txt file2.txt\n% git add file1.txt file2.txt \n% git diff -M --stat --cached\n file1.txt | 150 +++++++++++++++++++++++-------------------------------------\n file2.txt | 150 +++++++++++++++++++++++++++++++++++++-----------------------\n 2 files changed, 150 insertions(+), 150 deletions(-)\n\nI would expect at least to see\n\n file1.txt => file2.txt\n file2.txt => file1.txt\n\nif the contents are totally unchanged.\n\nAm I doing something wrong?\n\nThanks\n"},{"id":"123074","messageId":"7viqfmsoej.fsf@alter.siamese.dyndns.org","threadId":"20930","inReplyTo":"wavexx-2ECE7F.13171313092009@ger.gmane.org","subject":"Re: rename tracking and file-name swapping","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-13T18:14:12Z","receivedAt":"2009-09-13T18:14:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yuri D'Elia writes:\n\n> Hi everyone. Does rename tracking recognize two file names being swapped?\n>\n> % ls -l\n> total 24\n> -rw-rw-r-- 1 wavexx wavexx 5952 Sep 13 13:09 file1.txt\n> -rw-rw-r-- 1 wavexx wavexx 3330 Sep 13 13:09 file2.txt\n> % mv file1.txt file3.txt \n> % mv file2.txt file1.txt\n> % mv file3.txt file2.txt\n> % git add file1.txt file2.txt \n> % git diff -M --stat --cached\n> file1.txt | 150 +++++++++++++++++++++++-------------------------------------\n> file2.txt | 150 +++++++++++++++++++++++++++++++++++++-----------------------\n> 2 files changed, 150 insertions(+), 150 deletions(-)\n\nBy default, if the pathname that was present in the old version still\nappears in the new version, that path is not considered as a candiate\nfor rename detection. Only \"X used to be there but is gone\" and \"Y did\nnot exist but appeared\" are paired up and checked if they are similar.\n\nGive the command -B option, too, to break the filepair that does not\ndisappear.\n"},{"id":"123114","messageId":"1248088D-85CB-4335-AD8A-07DB5BAD1AAA@users.sf.net","threadId":"20930","inReplyTo":"7viqfmsoej.fsf@alter.siamese.dyndns.org","subject":"Re: rename tracking and file-name swapping","fromName":"Yuri D'Elia","fromEmail":"wavexx@users.sf.net","sentAt":"2009-09-13T21:13:52Z","receivedAt":"2009-09-13T21:13:52Z","isPatch":false,"sender":{"key":"wavexx@users.sf.net","avatar":null},"body":"On 13 Sep 2009, at 20:14, Junio C Hamano wrote:\n\n>> % mv file1.txt file3.txt\n>> % mv file2.txt file1.txt\n>> % mv file3.txt file2.txt\n>> % git add file1.txt file2.txt\n>> % git diff -M --stat --cached\n>> file1.txt | 150 ++++++++++++++++++++++ \n>> +-------------------------------------\n>> file2.txt | 150 ++++++++++++++++++++++++++++++++++++ \n>> +-----------------------\n>> 2 files changed, 150 insertions(+), 150 deletions(-)\n>\n> By default, if the pathname that was present in the old version still\n> appears in the new version, that path is not considered as a candiate\n> for rename detection. Only \"X used to be there but is gone\" and \"Y \n> did\n> not exist but appeared\" are paired up and checked if they are similar.\n>\n> Give the command -B option, too, to break the filepair that does not\n> disappear.\n\nThat does the trick. I'm curious, is there any other use for -B \nbesides rename handling?\nAny reason of why it isn't a default when --find-copies-harder is in \neffect?\n"},{"id":"123134","messageId":"4AADE655.60503@viscovery.net","threadId":"20930","inReplyTo":"1248088D-85CB-4335-AD8A-07DB5BAD1AAA@users.sf.net","subject":"Re: rename tracking and file-name swapping","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-09-14T06:44:37Z","receivedAt":"2009-09-14T06:44:37Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Yuri D'Elia schrieb:\n> On 13 Sep 2009, at 20:14, Junio C Hamano wrote:\n>> By default, if the pathname that was present in the old version still\n>> appears in the new version, that path is not considered as a candiate\n>> for rename detection. Only \"X used to be there but is gone\" and \"Y did\n>> not exist but appeared\" are paired up and checked if they are similar.\n>>\n>> Give the command -B option, too, to break the filepair that does not\n>> disappear.\n> \n> That does the trick. I'm curious, is there any other use for -B besides\n> rename handling?\n\nYes: It can make patches easier to read (just like -M and -C do) if a file\nwas completely rewritten. For example, look at b9dfe51c with and without\n-B, and also note the \"dissimilarity index\" in the diff header.\n\n-- Hannes\n"},{"id":"123201","messageId":"20090915123456.6117@nanako3.lavabit.com","threadId":"20930","inReplyTo":"7viqfmsoej.fsf@alter.siamese.dyndns.org","subject":"[Bug?] \"diff -B --color\" output doesn't show space errors","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-09-15T03:34:56Z","receivedAt":"2009-09-15T03:34:56Z","isPatch":false,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano \n\n> By default, if the pathname that was present in the old version still\n> appears in the new version, that path is not considered as a candiate\n> for rename detection. Only \"X used to be there but is gone\" and \"Y did\n> not exist but appeared\" are paired up and checked if they are similar.\n>\n> Give the command -B option, too, to break the filepair that does not\n> disappear.\n\nI wanted to try this -B option, and wrote a little test program.\n\nWhile it shows correctly that the file was rewritten, it doesn't\npoint out various whitespace mistakes in the file anymore.\n\nIs this a bug, or should I give some other options as well?\n\n-- >8 -- cut here -- 8< --\ngit init\n\ncat >file <<\"EOF\"\nThis is an article that will be\ncompletely rewritten in a\nlater commit.\nEOF\n\ngit add file\n\nsed -e \"s/T/\\t/g\" -e \"s/_/ /g\" >file <<\"EOF\"\nAn article was written but it was_\nlater rewritten to be_\na completely different text.\n_____\nAn article was written but it was_\nlater rewritten to be_\na completely different text.\n\nAn article was written but it was_\nlater rewritten to be_\na completely different text.\n\nWorse yet, the replacement text_\nintroduces a lot of\n_Twhite space errors_\nsuch as SP before HT and trailing\nwhitespaces, when the file was modified by the \nlater commit.\n\nAlso there are trailing empty lines at the end of the file.\n\n\n\nEOF\n\ngit diff --color\ngit diff --color -B\n# end of test program\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"123203","messageId":"7vvdjk95qi.fsf@alter.siamese.dyndns.org","threadId":"20930","inReplyTo":"20090915123456.6117@nanako3.lavabit.com","subject":"Re: [Bug?] \"diff -B --color\" output doesn't show space errors","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-15T04:44:37Z","receivedAt":"2009-09-15T04:44:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> I wanted to try this -B option, and wrote a little test program.\n>\n> While it shows correctly that the file was rewritten, it doesn't\n> point out various whitespace mistakes in the file anymore.\n\nAs you included the trailing blank lines, I assume you are running\n'next'.\n\nThe code to emit complete rewrite patch hasn't changed much since it was\nwritten, and I do not think it is aware of any whitespace error checking,\nlet alone the \"trailing blank lines\", which is pretty new for even the\nregular diff codepath.\n\nBut a more interesting thing about your test program is that it exposes to\na bug in the new code in next. Let me cook up a patch to fix that issue\nfirst, and then build probably a few more patches on it to add whitespace\nerror highlighting to the complete-rewrite codepath.\n"},{"id":"123204","messageId":"7vocpc94qy.fsf_-_@alter.siamese.dyndns.org","threadId":"20930","inReplyTo":"7vvdjk95qi.fsf@alter.siamese.dyndns.org","subject":"[PATCH] diff --whitespace: fix blank lines at end","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-09-15T05:05:57Z","receivedAt":"2009-09-15T05:05:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The earlier logic tried to colour any and all blank lines that were added\nbeyond the last blank line in the original, but this was very wrong. If\nyou added 96 blank lines, a non-blank line, and then 3 blank lines at the\nend, only the last 3 lines should trigger the error, not the earlier 96\nblank lines.\n\nWe need to also make sure that the lines are after the last non-blank line\nin the postimage as well before deciding to paint them.\n\nSigned-off-by: Junio C Hamano \n---\n\n * This is _not_ about colouring whitespace errors in -B output;\n that is a bit more involved change that needs a few more changes.\n\n diff.c | 74 +++++++++++++++++++++++++++++++++-------------\n t/t4019-diff-wserror.sh | 2 +-\n 2 files changed, 54 insertions(+), 22 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 2b285b8..63a3bfc 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -491,8 +491,10 @@ struct emit_callback {\n \tstruct xdiff_emit_state xm;\n \tint color_diff;\n \tunsigned ws_rule;\n-\tint blank_at_eof;\n+\tint blank_at_eof_in_preimage;\n+\tint blank_at_eof_in_postimage;\n \tint lno_in_preimage;\n+\tint lno_in_postimage;\n \tsane_truncate_fn truncate;\n \tconst char **label_path;\n \tstruct diff_words_data *diff_words;\n@@ -542,6 +544,17 @@ static void emit_line(FILE *file, const char *set, const char *reset, const char\n \t\tfputc('\\n', file);\n }\n \n+static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)\n+{\n+\tif (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&\n+\t ecbdata->blank_at_eof_in_preimage &&\n+\t ecbdata->blank_at_eof_in_postimage &&\n+\t ecbdata->blank_at_eof_in_preimage <= ecbdata->lno_in_preimage &&\n+\t ecbdata->blank_at_eof_in_postimage <= ecbdata->lno_in_postimage))\n+\t\treturn 0;\n+\treturn ws_blank_line(line + 1, len - 1, ecbdata->ws_rule);\n+}\n+\n static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)\n {\n \tconst char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);\n@@ -549,11 +562,8 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons\n \n \tif (!*ws)\n \t\temit_line(ecbdata->file, set, reset, line, len);\n-\telse if ((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&\n-\t\t ecbdata->blank_at_eof &&\n-\t\t (ecbdata->blank_at_eof <= ecbdata->lno_in_preimage) &&\n-\t\t ws_blank_line(line + 1, len - 1, ecbdata->ws_rule))\n-\t\t/* Blank line at EOF */\n+\telse if (new_blank_line_at_eof(ecbdata, line, len))\n+\t\t/* Blank line at EOF - paint '+' as well */\n \t\temit_line(ecbdata->file, ws, reset, line, len);\n \telse {\n \t\t/* Emit just the prefix, then the rest. */\n@@ -581,12 +591,19 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u\n \treturn allot - l;\n }\n \n-static int find_preimage_lno(const char *line)\n+static void find_lno(const char *line, struct emit_callback *ecbdata)\n {\n-\tchar *p = strchr(line, '-');\n+\tconst char *p;\n+\tecbdata->lno_in_preimage = 0;\n+\tecbdata->lno_in_postimage = 0;\n+\tp = strchr(line, '-');\n \tif (!p)\n-\t\treturn 0; /* should not happen */\n-\treturn strtol(p+1, NULL, 10);\n+\t\treturn; /* cannot happen */\n+\tecbdata->lno_in_preimage = strtol(p + 1, NULL, 10);\n+\tp = strchr(p, '+');\n+\tif (!p)\n+\t\treturn; /* cannot happen */\n+\tecbdata->lno_in_postimage = strtol(p + 1, NULL, 10);\n }\n \n static void fn_out_consume(void *priv, char *line, unsigned long len)\n@@ -613,7 +630,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \n \tif (line[0] == '@') {\n \t\tlen = sane_truncate_line(ecbdata, line, len);\n-\t\tecbdata->lno_in_preimage = find_preimage_lno(line);\n+\t\tfind_lno(line, ecbdata);\n \t\temit_line(ecbdata->file,\n \t\t\t diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),\n \t\t\t reset, line, len);\n@@ -651,10 +668,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)\n \t\t\tdiff_get_color(ecbdata->color_diff,\n \t\t\t\t line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);\n \t\tecbdata->lno_in_preimage++;\n+\t\tif (line[0] == ' ')\n+\t\t\tecbdata->lno_in_postimage++;\n \t\temit_line(ecbdata->file, color, reset, line, len);\n-\t\treturn;\n+\t} else {\n+\t\tecbdata->lno_in_postimage++;\n+\t\temit_add_line(reset, ecbdata, line, len);\n \t}\n-\temit_add_line(reset, ecbdata, line, len);\n }\n \n static char *pprint_rename(const char *a, const char *b)\n@@ -1470,16 +1490,23 @@ static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)\n \treturn cnt;\n }\n \n-static int adds_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, unsigned ws_rule)\n+static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,\n+\t\t\t struct emit_callback *ecbdata)\n {\n \tint l1, l2, at;\n+\tunsigned ws_rule = ecbdata->ws_rule;\n \tl1 = count_trailing_blank(mf1, ws_rule);\n \tl2 = count_trailing_blank(mf2, ws_rule);\n-\tif (l2 <= l1)\n-\t\treturn 0;\n-\t/* starting where? */\n+\tif (l2 <= l1) {\n+\t\tecbdata->blank_at_eof_in_preimage = 0;\n+\t\tecbdata->blank_at_eof_in_postimage = 0;\n+\t\treturn;\n+\t}\n \tat = count_lines(mf1->ptr, mf1->size);\n-\treturn (at - l1) + 1; /* the line number counts from 1 */\n+\tecbdata->blank_at_eof_in_preimage = (at - l1) + 1;\n+\n+\tat = count_lines(mf2->ptr, mf2->size);\n+\tecbdata->blank_at_eof_in_postimage = (at - l2) + 1;\n }\n \n static void builtin_diff(const char *name_a,\n@@ -1572,8 +1599,7 @@ static void builtin_diff(const char *name_a,\n \t\tecbdata.found_changesp = &o->found_changes;\n \t\tecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);\n \t\tif (ecbdata.ws_rule & WS_BLANK_AT_EOF)\n-\t\t\tecbdata.blank_at_eof =\n-\t\t\t\tadds_blank_at_eof(&mf1, &mf2, ecbdata.ws_rule);\n+\t\t\tcheck_blank_at_eof(&mf1, &mf2, &ecbdata);\n \t\tecbdata.file = o->file;\n \t\txpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;\n \t\txecfg.ctxlen = o->context;\n@@ -1699,7 +1725,13 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \t\txdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);\n \n \t\tif (data.ws_rule & WS_BLANK_AT_EOF) {\n-\t\t\tint blank_at_eof = adds_blank_at_eof(&mf1, &mf2, data.ws_rule);\n+\t\t\tstruct emit_callback ecbdata;\n+\t\t\tint blank_at_eof;\n+\n+\t\t\tecbdata.ws_rule = data.ws_rule;\n+\t\t\tcheck_blank_at_eof(&mf1, &mf2, &ecbdata);\n+\t\t\tblank_at_eof = ecbdata.blank_at_eof_in_preimage;\n+\n \t\t\tif (blank_at_eof) {\n \t\t\t\tstatic char *err;\n \t\t\t\tif (!err)\ndiff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh\nindex 1e75f1a..3a3663f 100755\n--- a/t/t4019-diff-wserror.sh\n+++ b/t/t4019-diff-wserror.sh\n@@ -193,7 +193,7 @@ test_expect_success 'do not color trailing cr in context' '\n test_expect_success 'color new trailing blank lines' '\n \t{ echo a; echo b; echo; echo; } >x &&\n \tgit add x &&\n-\t{ echo a; echo; echo; echo; echo; } >x &&\n+\t{ echo a; echo; echo; echo; echo c; echo; echo; echo; echo; } >x &&\n \tgit diff --color x >output &&\n \tcnt=$(grep \"${blue_grep}\" output | wc -l) &&\n \ttest $cnt = 2\n-- \n1.6.5.rc1.54.g4aad\n"}]}