{"thread":{"id":"27400","subject":"Re: [FEATURE] difftool to provide meaningful names for temporary files","startedAt":"2011-05-18T21:54:02Z","lastAt":"2011-05-20T04:32:04Z","messageCount":2,"participants":["Eugene Sajine","David Aguilar"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"168163","messageId":"BANLkTimn4sYCFrLdPWOypwVNUBR5kMVqxg@mail.gmail.com","threadId":"27400","inReplyTo":"BANLkTikC6MhvEiw=7JKscN5iOPFzVGxJzA@mail.gmail.com","subject":"Re: [FEATURE] difftool to provide meaningful names for temporary files","fromName":"Eugene Sajine","fromEmail":"euguess@gmail.com","sentAt":"2011-05-18T21:54:02Z","receivedAt":"2011-05-18T21:54:02Z","isPatch":false,"sender":{"key":"euguess@gmail.com","avatar":null},"body":"Hi,\n\nI noticed many times already that it is at times confusing to\ndetermine for new users which file versions are shown where in the\ngraphical told like gvimdiff or kompare.\n\nThe problem is that when difftool machinery creates a temporary files\nto show in the editor it is creating them with some random hashes in\nthe beginning of the file name.\n\nIs it possible to make the difftool and mergetool smarter to have them\nto create those temporary files with meaningful names, f.e.:\n\nIf I specify:\nGit difftool master dev\nIt will create the files with master and dev prefixes correspondingly,\nit will take the two points names and use them in temp file names.\nMaster_file.txt vs dev_file.txt\n\nIf I specify:\nGit difftool\nIt could take HEAD and local as name prefixes\n\nSame for mergetool where it could use HEAD and incoming branch name as\nprefixes or something like that.\n\nDoes it make sense?\n\nThanks,\nEugene\n"},{"id":"168294","messageId":"20110520043202.GC13582@gmail.com","threadId":"27400","inReplyTo":"BANLkTimn4sYCFrLdPWOypwVNUBR5kMVqxg@mail.gmail.com","subject":"Re: [FEATURE] difftool to provide meaningful names for temporary files","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2011-05-20T04:32:04Z","receivedAt":"2011-05-20T04:32:04Z","isPatch":false,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Wed, May 18, 2011 at 05:54:02PM -0400, Eugene Sajine wrote:\n> Hi,\n> \n> I noticed many times already that it is at times confusing to\n> determine for new users which file versions are shown where in the\n> graphical told like gvimdiff or kompare.\n> \n> The problem is that when difftool machinery creates a temporary files\n> to show in the editor it is creating them with some random hashes in\n> the beginning of the file name.\n> \n> Is it possible to make the difftool and mergetool smarter to have them\n> to create those temporary files with meaningful names, f.e.:\n> \n> If I specify:\n> Git difftool master dev\n> It will create the files with master and dev prefixes correspondingly,\n> it will take the two points names and use them in temp file names.\n> Master_file.txt vs dev_file.txt\n> \n> If I specify:\n> Git difftool\n> It could take HEAD and local as name prefixes\n> \n> Same for mergetool where it could use HEAD and incoming branch name as\n> prefixes or something like that.\n> \n> Does it make sense?\n\nThat makes sense, but part of the reason why we use\nmkstemp() is to make the paths random.\n\nThis is better then what it was like before, though --\nthey used to be completely random! ;-)\n\nI think the randomness is a little bit of a security feature.\nI wouldn't want to discourage you from trying to make it better,\nthough.  Here's how I first improved it.  Maybe you can take\nthis as a starting point for making it better?\n\n# david@lustrous:~/src/git (master)\n% git show 003b33a8ad686ee4a0d0b36635bfd6aba940b24a\ncommit 003b33a8ad686ee4a0d0b36635bfd6aba940b24a\nAuthor: David Aguilar <davvid@gmail.com>\nDate:   Sun May 31 01:35:52 2009 -0700\n\n    diff: generate pretty filenames in prep_temp_blob()\n    \n    Naturally, prep_temp_blob() did not care about filenames.\n    As a result, GIT_EXTERNAL_DIFF and textconv generated\n    filenames such as \".diff_XXXXXX\".\n    \n    This modifies prep_temp_blob() to generate user-friendly\n    filenames when creating temporary files.\n    \n    Diffing \"name.ext\" now generates \"XXXXXX_name.ext\".\n    \n    Signed-off-by: David Aguilar <davvid@gmail.com>\n    Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\ndiff --git a/cache.h b/cache.h\nindex b8503ad..871c984 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -614,6 +614,8 @@ extern int is_empty_blob_sha1(const unsigned char *sha1);\n \n int git_mkstemp(char *path, size_t n, const char *template);\n \n+int git_mkstemps(char *path, size_t n, const char *template, int suffix_len);\n+\n /*\n  * NOTE NOTE NOTE!!\n  *\ndiff --git a/diff.c b/diff.c\nindex dcfbcb0..4d0a5b9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1964,8 +1964,16 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp,\n {\n \tint fd;\n \tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf template = STRBUF_INIT;\n+\tchar *path_dup = xstrdup(path);\n+\tconst char *base = basename(path_dup);\n \n-\tfd = git_mkstemp(temp->tmp_path, PATH_MAX, \".diff_XXXXXX\");\n+\t/* Generate \"XXXXXX_basename.ext\" */\n+\tstrbuf_addstr(&template, \"XXXXXX_\");\n+\tstrbuf_addstr(&template, base);\n+\n+\tfd = git_mkstemps(temp->tmp_path, PATH_MAX, template.buf,\n+\t\t\tstrlen(base) + 1);\n \tif (fd < 0)\n \t\tdie(\"unable to create temp-file: %s\", strerror(errno));\n \tif (convert_to_working_tree(path,\n@@ -1981,6 +1989,8 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp,\n \ttemp->hex[40] = 0;\n \tsprintf(temp->mode, \"%06o\", mode);\n \tstrbuf_release(&buf);\n+\tstrbuf_release(&template);\n+\tfree(path_dup);\n }\n \n static struct diff_tempfile *prepare_temp_file(const char *name,\ndiff --git a/path.c b/path.c\nindex 8a0a674..047fdb0 100644\n--- a/path.c\n+++ b/path.c\n@@ -139,6 +139,22 @@ int git_mkstemp(char *path, size_t len, const char *template)\n \treturn mkstemp(path);\n }\n \n+/* git_mkstemps() - create tmp file with suffix honoring TMPDIR variable. */\n+int git_mkstemps(char *path, size_t len, const char *template, int suffix_len)\n+{\n+\tconst char *tmp;\n+\tsize_t n;\n+\n+\ttmp = getenv(\"TMPDIR\");\n+\tif (!tmp)\n+\t\ttmp = \"/tmp\";\n+\tn = snprintf(path, len, \"%s/%s\", tmp, template);\n+\tif (len <= n) {\n+\t\terrno = ENAMETOOLONG;\n+\t\treturn -1;\n+\t}\n+\treturn mkstemps(path, suffix_len);\n+}\n \n int validate_headref(const char *path)\n {\ndiff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh\nindex 0720001..4ea42e0 100755\n--- a/t/t4020-diff-external.sh\n+++ b/t/t4020-diff-external.sh\n@@ -136,6 +136,15 @@ test_expect_success 'GIT_EXTERNAL_DIFF with more than one changed files' '\n \tGIT_EXTERNAL_DIFF=echo git diff\n '\n \n+test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' '\n+\ttouch file.ext &&\n+\tgit add file.ext &&\n+\techo with extension > file.ext &&\n+\tGIT_EXTERNAL_DIFF=echo git diff file.ext | grep ......_file\\.ext &&\n+\tgit update-index --force-remove file.ext &&\n+\trm file.ext\n+'\n+\n echo \"#!$SHELL_PATH\" >fake-diff.sh\n cat >> fake-diff.sh <<\\EOF\n cat $2 >> crlfed.txt\n\n-- \n\t\t\t\t\tDavid\n"}]}