{"thread":{"id":"34120","subject":"[PATCH jx/clean-interactive] t0060: skip a few relative_path tests on Windows","startedAt":"2013-06-13T08:23:34Z","lastAt":"2013-06-25T19:23:44Z","messageCount":33,"participants":["Johannes Sixt","Jiang Xin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"220700","messageId":"51B98186.2020100@viscovery.net","threadId":"34120","inReplyTo":null,"subject":"[PATCH jx/clean-interactive] t0060: skip a few relative_path tests on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-06-13T08:23:34Z","receivedAt":"2013-06-13T08:23:34Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nThe bash on Windows rewrites paths that look like absolute POSIX paths\nwhen they are a command-line argument of a regular Windows program, such\nas git and the test helpers. As a consequence, the actual tests performed\nare not what the tests scripts expect.\n\nThe tests that need *not* be skipped are those where the two paths passed\nto 'test-path-utils relative_path' have the same prefix and the result is\nexpected to be a relative path. This is because the rewriting changes\n\"/a/b\" to \"D:/Src/MSysGit/a/b\", and when both inputs are extended the same\nway, this just cancels out in the relative path computation.\n\nSigned-off-by: Johannes Sixt \n---\n t/t0060-path-utils.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex dfe4747..4deec52 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -190,15 +190,15 @@ test_expect_success SYMLINKS 'real path works on\nsymlinks' '\n\n relative_path /a/b/c/\t/a/b/\t\tc/\n relative_path /a/b/c/\t/a/b\t\tc/\n-relative_path /a//b//c/\t//a/b//\t\tc/\n+relative_path /a//b//c/\t//a/b//\t\tc/\tPOSIX\n relative_path /a/b\t/a/b\t\t./\n relative_path /a/b/\t/a/b\t\t./\n relative_path /a\t/a/b\t\t../\n relative_path /\t\t/a/b/\t\t../../\n relative_path /a/c\t/a/b/\t\t../c\n relative_path /a/c\t/a/b\t\t../c\n-relative_path /a/b\t\"\"\t/a/b\n-relative_path /a/b \t\"\"\t/a/b\n+relative_path /a/b\t\"\"\t/a/b\tPOSIX\n+relative_path /a/b \t\"\"\t/a/b\tPOSIX\n relative_path \"\"\t/a/b\t\t./\n relative_path \"\"\t\"\"\t./\n relative_path \"\"\t\"\"\t./\n-- \n1.8.3.1.1670.g1dbc49e\n"},{"id":"220704","messageId":"CANYiYbHQ307TroSd6Lk=5zg68jdKxGeA4N=hWGfaz1W29NhNVw@mail.gmail.com","threadId":"34120","inReplyTo":"51B98186.2020100@viscovery.net","subject":"Re: [PATCH jx/clean-interactive] t0060: skip a few relative_path tests on Windows","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-13T09:40:21Z","receivedAt":"2013-06-13T09:40:21Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/6/13 Johannes Sixt :\n> From: Johannes Sixt \n>\n> The bash on Windows rewrites paths that look like absolute POSIX paths\n> when they are a command-line argument of a regular Windows program, such\n> as git and the test helpers. As a consequence, the actual tests performed\n> are not what the tests scripts expect.\n>\n> The tests that need *not* be skipped are those where the two paths passed\n> to 'test-path-utils relative_path' have the same prefix and the result is\n> expected to be a relative path. This is because the rewriting changes\n> \"/a/b\" to \"D:/Src/MSysGit/a/b\", and when both inputs are extended the same\n> way, this just cancels out in the relative path computation.\n>\n> Signed-off-by: Johannes Sixt \n> ---\n> t/t0060-path-utils.sh | 6 +++---\n> 1 file changed, 3 insertions(+), 3 deletions(-)\n>\n\nThank you for pointing out this cross-platform issue.\nI test your patch on Mac OS X, but not test it on msys yet.\nSince this issue is from the very first commit of this series\nand this series are still in pu, so I move your patch as\npatch 02/16 of this series.\n\nYou can get the update series in my clone on GitHub:\n\n $ git remote add jiangxin git://github.com/jiangxin/git.git\n $ git fetch jiangxin jx/clean-interactive\n $ git log --oneline -16 jiangxin/jx/clean-interactive\n\n c646c test: add t7301 for git-clean--interactive\n 92d4a git-clean: add documentation for interactive git-clean\n 22e3d git-clean: add ask each interactive action\n 5fcb8 git-clean: add select by numbers interactive action\n 77ef8e git-clean: add filter by pattern interactive action\n 02327 git-clean: use a git-add-interactive compatible UI\n 2322 git-clean: add colors to interactive git-clean\n 06fad git-clean: show items of del_list in columns\n 6ae8b git-clean: add support for -i/--interactive\n 1eeb5 git-clean: refactor git-clean into two phases\n 3f903 Refactor write_name_quoted_relative, remove unused params\n 8ccdf Refactor quote_path_relative, remove unused params\n 13da5e quote.c: remove path_relative, use relative_path instead\n 1208ee path.c: refactor relative_path(), not only strip prefix\n 95b06 t0060: skip a few relative_path tests on Windows\n 22247 test: add test cases for relative_path\n\nAnd commit 95b06 may squash to previous commit 22247.\n\n-- \nJiang Xin\n"},{"id":"221802","messageId":"cover.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"CANYiYbHQ307TroSd6Lk=5zg68jdKxGeA4N=hWGfaz1W29NhNVw@mail.gmail.com","subject":"[PATCH v14 00/16] Interactive git-clean","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:24Z","receivedAt":"2013-06-24T15:21:24Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Johannes found that some relative_path tests should be skipped on Windows.\nSee this thread:\n\n http://thread.gmane.org/gmane.comp.version-control.git/227706\n\nIn this reroll,\n\n * I squash Johannes's patch into patch 01/16, and seven relative_path\n test cases are skipped by marking with POSIX.\n \n * In patch 02/16, 4 test cases can run correctly as the refactor of\n relative_path.\n \n * In patch 16/16, most of the previous skiped test cases can run with\n the help of the new utiliy \"test-path-utils mingw_path /abs/path/\".\n\nJiang Xin (16):\n test: add test cases for relative_path\n path.c: refactor relative_path(), not only strip prefix\n quote.c: remove path_relative, use relative_path instead\n Refactor quote_path_relative, remove unused params\n Refactor write_name_quoted_relative, remove unused params\n git-clean: refactor git-clean into two phases\n git-clean: add support for -i/--interactive\n git-clean: show items of del_list in columns\n git-clean: add colors to interactive git-clean\n git-clean: use a git-add-interactive compatible UI\n git-clean: add filter by pattern interactive action\n git-clean: add select by numbers interactive action\n git-clean: add ask each interactive action\n git-clean: add documentation for interactive git-clean\n test: add t7301 for git-clean--interactive\n test: run testcases with POSIX absolute paths on Windows\n\n Documentation/config.txt | 21 +-\n Documentation/git-clean.txt | 71 +++-\n builtin/clean.c | 778 +++++++++++++++++++++++++++++++++++++++++--\n builtin/grep.c | 5 +-\n builtin/ls-files.c | 16 +-\n cache.h | 2 +-\n path.c | 112 +++++--\n quote.c | 65 +---\n quote.h | 7 +-\n setup.c | 5 +-\n t/t0060-path-utils.sh | 88 +++--\n t/t7301-clean-interactive.sh | 439 ++++++++++++++++++++++++\n test-path-utils.c | 32 ++\n wt-status.c | 17 +-\n 14 files changed, 1487 insertions(+), 171 deletions(-)\n create mode 100755 t/t7301-clean-interactive.sh\n\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221803","messageId":"85c9aa4d05dba4320a7cc85eae7570e0f450e58e.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 01/16] test: add test cases for relative_path","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:25Z","receivedAt":"2013-06-24T15:21:25Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Add subcommand \"relative_path\" in test-path-utils, and add test cases\nin t0060.\n\nJohannes tested this commit on Windows, and found that some relative_path\ntests should be skipped on Windows. This is because the bash on Windows\nrewrites arguments of regular Windows programs, such as git and the\ntest helpers, if the arguments look like absolute POSIX paths. As a\nconsequence, the actual tests performed are not what the tests scripts\nexpect.\n\nThe tests that need *not* be skipped are those where the two paths passed\nto 'test-path-utils relative_path' have the same prefix and the result is\nexpected to be a relative path. This is because the rewriting changes\n\"/a/b\" to \"D:/Src/MSysGit/a/b\", and when both inputs are extended the same\nway, this just cancels out in the relative path computation.\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Johannes Sixt \nSigned-off-by: Junio C Hamano \n---\n t/t0060-path-utils.sh | 26 ++++++++++++++++++++++++++\n test-path-utils.c | 25 +++++++++++++++++++++++++\n 2 files changed, 51 insertions(+)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 09a42..7e258 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -12,6 +12,11 @@ norm_path() {\n \t\"test \\\"\\$(test-path-utils normalize_path_copy '$1')\\\" = '$2'\"\n }\n \n+relative_path() {\n+\ttest_expect_success $4 \"relative path: $1 $2 => $3\" \\\n+\t\"test \\\"\\$(test-path-utils relative_path '$1' '$2')\\\" = '$3'\"\n+}\n+\n # On Windows, we are using MSYS's bash, which mangles the paths.\n # Absolute paths are anchored at the MSYS installation directory,\n # which means that the path / accounts for this many characters:\n@@ -183,4 +188,25 @@ test_expect_success SYMLINKS 'real path works on symlinks' '\n \ttest \"$sym\" = \"$(test-path-utils real_path \"$dir2/syml\")\"\n '\n \n+relative_path /a/b/c/\t/a/b/\t\tc/\n+relative_path /a/b/c/\t/a/b\t\tc/\n+relative_path /a//b//c/\t//a/b//\t\tc/\tPOSIX\n+relative_path /a/b\t/a/b\t\t.\n+relative_path /a/b/\t/a/b\t\t.\n+relative_path /a\t/a/b\t\t/a\tPOSIX\n+relative_path /\t\t/a/b/\t\t/\tPOSIX\n+relative_path /a/c\t/a/b/\t\t/a/c\tPOSIX\n+relative_path /a/c\t/a/b\t\t/a/c\tPOSIX\n+relative_path /a/b\t\"\"\t/a/b\tPOSIX\n+relative_path /a/b \t\"\"\t/a/b\tPOSIX\n+relative_path \"\"\t/a/b\t\t\"(empty)\"\n+relative_path \"\"\t\"\"\t\"(empty)\"\n+relative_path \"\"\t\"\"\t\"(empty)\"\n+relative_path \"\"\t\"\"\t\"(null)\"\n+relative_path \"\"\t\"\"\t\"(null)\"\n+\n+test_expect_failure 'relative path: /a/b => segfault' '\n+\ttest-path-utils relative_path \"\" \"/a/b\"\n+'\n+\n test_done\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex 0092cb..dcc530 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -28,6 +28,19 @@ static int normalize_ceiling_entry(struct string_list_item *item, void *unused)\n \treturn 1;\n }\n \n+static void normalize_argv_string(const char **var, const char *input)\n+{\n+\tif (!strcmp(input, \"\"))\n+\t\t*var = NULL;\n+\telse if (!strcmp(input, \"\"))\n+\t\t*var = \"\";\n+\telse\n+\t\t*var = input;\n+\n+\tif (*var && (**var == '<' || **var == '('))\n+\t\tdie(\"Bad value: %s\\n\", input);\n+}\n+\n int main(int argc, char **argv)\n {\n \tif (argc == 3 && !strcmp(argv[1], \"normalize_path_copy\")) {\n@@ -103,6 +116,18 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n+\tif (argc == 4 && !strcmp(argv[1], \"relative_path\")) {\n+\t\tconst char *abs, *base, *rel;\n+\t\tnormalize_argv_string(&abs, argv[2]);\n+\t\tnormalize_argv_string(&base, argv[3]);\n+\t\trel = relative_path(abs, base);\n+\t\tif (!rel)\n+\t\t\tputs(\"(null)\");\n+\t\telse\n+\t\t\tputs(strlen(rel) > 0 ? rel : \"(empty)\");\n+\t\treturn 0;\n+\t}\n+\n \tfprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n \t\targv[1] ? argv[1] : \"(there was none)\");\n \treturn 1;\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221804","messageId":"28a573cdde51da2f83a8692229fd360fec0e3783.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 02/16] path.c: refactor relative_path(), not only strip prefix","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:26Z","receivedAt":"2013-06-24T15:21:26Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Original design of relative_path() is simple, just strip the prefix\n(*base) from the absolute path (*abs). In most cases, we need a real\nrelative path, such as: ../foo, ../../bar. That's why there is another\nreimplementation (path_relative()) in quote.c.\n\nBorrowed some codes from path_relative() in quote.c to refactor\nrelative_path() in path.c, so that it could return real relative path,\nand user can reuse this function without reimplement his/her own.\nThe function path_relative() in quote.c will be substituted, and I\nwould use the new relative_path() function when implement the\ninteractive git-clean later.\n\nDifferent results for relative_path() before and after this refactor:\n\n abs path base path relative (original) relative (refactor)\n ======== ========= =================== ===================\n /a/b/c/ /a/b c/ c/\n /a/b//c/ //a///b/ c/ c/\n /a/b /a/b . ./\n /a/b/ /a/b . ./\n /a /a/b/ /a ../\n / /a/b/ / ../../\n /a/c /a/b/ /a/c ../c\n /a/b (empty) /a/b /a/b\n /a/b (null) /a/b /a/b\n (empty) /a/b (empty) ./\n (null) (empty) (null) ./\n (null) /a/b (segfault) ./\n\nYou may notice that return value \".\" has been changed to \"./\".\nIt is because:\n\n * Function quote_path_relative() in quote.c will show the relative\n path as \"./\" if abs(in) and base(prefix) are the same.\n\n * Function relative_path() is called only once (in setup.c), and\n it will be OK for the return value as \"./\" instead of \".\".\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Junio C Hamano \n---\n cache.h | 2 +-\n path.c | 112 +++++++++++++++++++++++++++++++++++++++-----------\n setup.c | 5 ++-\n t/t0060-path-utils.sh | 27 ++++++------\n test-path-utils.c | 4 +-\n 5 files changed, 107 insertions(+), 43 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex dd0fb..2f10c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -758,7 +758,7 @@ int is_directory(const char *);\n const char *real_path(const char *path);\n const char *real_path_if_valid(const char *path);\n const char *absolute_path(const char *path);\n-const char *relative_path(const char *abs, const char *base);\n+const char *relative_path(const char *abs, const char *base, struct strbuf *sb);\n int normalize_path_copy(char *dst, const char *src);\n int longest_ancestor_length(const char *path, struct string_list *prefixes);\n char *strip_path_suffix(const char *path, const char *suffix);\ndiff --git a/path.c b/path.c\nindex 04ff..8ff16 100644\n--- a/path.c\n+++ b/path.c\n@@ -441,42 +441,104 @@ int adjust_shared_perm(const char *path)\n \treturn 0;\n }\n \n-const char *relative_path(const char *abs, const char *base)\n+/*\n+ * Give relative path for abs to base.\n+ *\n+ * The strbuf may or may not be used, so do not assume it contains the\n+ * returned path.\n+ */\n+const char *relative_path(const char *abs, const char *base,\n+\t\t\t struct strbuf *sb)\n {\n-\tstatic char buf[PATH_MAX + 1];\n-\tint i = 0, j = 0;\n-\n-\tif (!base || !base[0])\n+\tint abs_off, base_off, i, j;\n+\tint abs_len, base_len;\n+\n+\tabs_len = abs ? strlen(abs) : 0;\n+\tbase_len = base ? strlen(base) : 0;\n+\tabs_off = 0;\n+\tbase_off = 0;\n+\ti = 0;\n+\tj = 0;\n+\n+\tif (!abs_len)\n+\t\treturn \"./\";\n+\telse if (!base_len)\n \t\treturn abs;\n-\twhile (base[i]) {\n+\n+\twhile (i < base_len && j < abs_len && base[i] == abs[j]) {\n \t\tif (is_dir_sep(base[i])) {\n-\t\t\tif (!is_dir_sep(abs[j]))\n-\t\t\t\treturn abs;\n \t\t\twhile (is_dir_sep(base[i]))\n \t\t\t\ti++;\n \t\t\twhile (is_dir_sep(abs[j]))\n \t\t\t\tj++;\n-\t\t\tcontinue;\n-\t\t} else if (abs[j] != base[i]) {\n+\t\t\tbase_off = i;\n+\t\t\tabs_off = j;\n+\t\t} else {\n+\t\t\ti++;\n+\t\t\tj++;\n+\t\t}\n+\t}\n+\n+\tif (\n+\t /* base seems like prefix of abs */\n+\t i >= base_len &&\n+\t /*\n+\t * but \"/foo\" is not a prefix of \"/foobar\"\n+\t * (i.e. base not end with '/')\n+\t */\n+\t base_off < base_len) {\n+\t\tif (j >= abs_len) {\n+\t\t\t/* abs=\"/a/b\", base=\"/a/b\" */\n+\t\t\tabs_off = abs_len;\n+\t\t} else if (is_dir_sep(abs[j])) {\n+\t\t\t/* abs=\"/a/b/c\", base=\"/a/b\" */\n+\t\t\twhile (is_dir_sep(abs[j]))\n+\t\t\t\tj++;\n+\t\t\tabs_off = j;\n+\t\t} else {\n+\t\t\t/* abs=\"/a/bbb/c\", base=\"/a/b\" */\n+\t\t\ti = base_off;\n+\t\t}\n+\t} else if (\n+\t\t /* abs is short than base (prefix of base) */\n+\t\t j >= abs_len &&\n+\t\t /* abs not end with '/' */\n+\t\t abs_off < abs_len) {\n+\t\tif (is_dir_sep(base[i])) {\n+\t\t\t/* abs=\"/a/b\", base=\"/a/b/c/\" */\n+\t\t\twhile (is_dir_sep(base[i]))\n+\t\t\t\ti++;\n+\t\t\tabs_off = abs_len;\n+\t\t}\n+\t}\n+\tabs += abs_off;\n+\tabs_len -= abs_off;\n+\n+\tif (i >= base_len) {\n+\t\tif (!abs_len)\n+\t\t\treturn \"./\";\n+\t\telse\n \t\t\treturn abs;\n+\t}\n+\n+\tstrbuf_reset(sb);\n+\tstrbuf_grow(sb, abs_len);\n+\n+\twhile (i < base_len) {\n+\t\tif (is_dir_sep(base[i])) {\n+\t\t\tstrbuf_addstr(sb, \"../\");\n+\t\t\twhile (is_dir_sep(base[i]))\n+\t\t\t\ti++;\n+\t\t\tcontinue;\n \t\t}\n \t\ti++;\n-\t\tj++;\n \t}\n-\tif (\n-\t /* \"/foo\" is a prefix of \"/foo\" */\n-\t abs[j] &&\n-\t /* \"/foo\" is not a prefix of \"/foobar\" */\n-\t !is_dir_sep(base[i-1]) && !is_dir_sep(abs[j])\n-\t )\n-\t\treturn abs;\n-\twhile (is_dir_sep(abs[j]))\n-\t\tj++;\n-\tif (!abs[j])\n-\t\tstrcpy(buf, \".\");\n-\telse\n-\t\tstrcpy(buf, abs + j);\n-\treturn buf;\n+\tif (!is_dir_sep(base[base_len - 1]))\n+\t\tstrbuf_addstr(sb, \"../\");\n+\n+\tstrbuf_addstr(sb, abs);\n+\n+\treturn sb->buf;\n }\n \n /*\ndiff --git a/setup.c b/setup.c\nindex 94c1e..0d9ea 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -360,6 +360,7 @@ int is_inside_work_tree(void)\n \n void setup_work_tree(void)\n {\n+\tstruct strbuf sb = STRBUF_INIT;\n \tconst char *work_tree, *git_dir;\n \tstatic int initialized = 0;\n \n@@ -379,8 +380,10 @@ void setup_work_tree(void)\n \tif (getenv(GIT_WORK_TREE_ENVIRONMENT))\n \t\tsetenv(GIT_WORK_TREE_ENVIRONMENT, \".\", 1);\n \n-\tset_git_dir(relative_path(git_dir, work_tree));\n+\tset_git_dir(relative_path(git_dir, work_tree, &sb));\n \tinitialized = 1;\n+\n+\tstrbuf_release(&sb);\n }\n \n static int check_repository_format_gently(const char *gitdir, int *nongit_ok)\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 7e258..4deec 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -191,22 +191,19 @@ test_expect_success SYMLINKS 'real path works on symlinks' '\n relative_path /a/b/c/\t/a/b/\t\tc/\n relative_path /a/b/c/\t/a/b\t\tc/\n relative_path /a//b//c/\t//a/b//\t\tc/\tPOSIX\n-relative_path /a/b\t/a/b\t\t.\n-relative_path /a/b/\t/a/b\t\t.\n-relative_path /a\t/a/b\t\t/a\tPOSIX\n-relative_path /\t\t/a/b/\t\t/\tPOSIX\n-relative_path /a/c\t/a/b/\t\t/a/c\tPOSIX\n-relative_path /a/c\t/a/b\t\t/a/c\tPOSIX\n+relative_path /a/b\t/a/b\t\t./\n+relative_path /a/b/\t/a/b\t\t./\n+relative_path /a\t/a/b\t\t../\n+relative_path /\t\t/a/b/\t\t../../\n+relative_path /a/c\t/a/b/\t\t../c\n+relative_path /a/c\t/a/b\t\t../c\n relative_path /a/b\t\"\"\t/a/b\tPOSIX\n relative_path /a/b \t\"\"\t/a/b\tPOSIX\n-relative_path \"\"\t/a/b\t\t\"(empty)\"\n-relative_path \"\"\t\"\"\t\"(empty)\"\n-relative_path \"\"\t\"\"\t\"(empty)\"\n-relative_path \"\"\t\"\"\t\"(null)\"\n-relative_path \"\"\t\"\"\t\"(null)\"\n-\n-test_expect_failure 'relative path: /a/b => segfault' '\n-\ttest-path-utils relative_path \"\" \"/a/b\"\n-'\n+relative_path \"\"\t/a/b\t\t./\n+relative_path \"\"\t\"\"\t./\n+relative_path \"\"\t\"\"\t./\n+relative_path \"\"\t\"\"\t./\n+relative_path \"\"\t\"\"\t./\n+relative_path \"\"\t/a/b\t\t./\n \n test_done\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex dcc530..95ef4 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -117,14 +117,16 @@ int main(int argc, char **argv)\n \t}\n \n \tif (argc == 4 && !strcmp(argv[1], \"relative_path\")) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n \t\tconst char *abs, *base, *rel;\n \t\tnormalize_argv_string(&abs, argv[2]);\n \t\tnormalize_argv_string(&base, argv[3]);\n-\t\trel = relative_path(abs, base);\n+\t\trel = relative_path(abs, base, &sb);\n \t\tif (!rel)\n \t\t\tputs(\"(null)\");\n \t\telse\n \t\t\tputs(strlen(rel) > 0 ? rel : \"(empty)\");\n+\t\tstrbuf_release(&sb);\n \t\treturn 0;\n \t}\n \n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221805","messageId":"654b2387c27470c3dc81169e8f43c231aa9504ce.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 03/16] quote.c: remove path_relative, use relative_path instead","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:27Z","receivedAt":"2013-06-24T15:21:27Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Since there is an enhanced version of relative_path() in path.c,\nremove duplicate counterpart path_relative() in quote.c.\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Junio C Hamano \n---\n quote.c | 55 ++-----------------------------------------------------\n 1 file changed, 2 insertions(+), 53 deletions(-)\n\ndiff --git a/quote.c b/quote.c\nindex 91122..64ff3 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -312,75 +312,24 @@ void write_name_quotedpfx(const char *pfx, size_t pfxlen,\n \tfputc(terminator, fp);\n }\n \n-static const char *path_relative(const char *in, int len,\n-\t\t\t\t struct strbuf *sb, const char *prefix,\n-\t\t\t\t int prefix_len);\n-\n void write_name_quoted_relative(const char *name, size_t len,\n \t\t\t\tconst char *prefix, size_t prefix_len,\n \t\t\t\tFILE *fp, int terminator)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \n-\tname = path_relative(name, len, &sb, prefix, prefix_len);\n+\tname = relative_path(name, prefix, &sb);\n \twrite_name_quoted(name, fp, terminator);\n \n \tstrbuf_release(&sb);\n }\n \n-/*\n- * Give path as relative to prefix.\n- *\n- * The strbuf may or may not be used, so do not assume it contains the\n- * returned path.\n- */\n-static const char *path_relative(const char *in, int len,\n-\t\t\t\t struct strbuf *sb, const char *prefix,\n-\t\t\t\t int prefix_len)\n-{\n-\tint off, i;\n-\n-\tif (len < 0)\n-\t\tlen = strlen(in);\n-\tif (prefix_len < 0) {\n-\t\tif (prefix)\n-\t\t\tprefix_len = strlen(prefix);\n-\t\telse\n-\t\t\tprefix_len = 0;\n-\t}\n-\n-\toff = 0;\n-\ti = 0;\n-\twhile (i < prefix_len && i < len && prefix[i] == in[i]) {\n-\t\tif (prefix[i] == '/')\n-\t\t\toff = i + 1;\n-\t\ti++;\n-\t}\n-\tin += off;\n-\tlen -= off;\n-\n-\tif (i >= prefix_len)\n-\t\treturn in;\n-\n-\tstrbuf_reset(sb);\n-\tstrbuf_grow(sb, len);\n-\n-\twhile (i < prefix_len) {\n-\t\tif (prefix[i] == '/')\n-\t\t\tstrbuf_addstr(sb, \"../\");\n-\t\ti++;\n-\t}\n-\tstrbuf_add(sb, in, len);\n-\n-\treturn sb->buf;\n-}\n-\n /* quote path as relative to the given prefix */\n char *quote_path_relative(const char *in, int len,\n \t\t\t struct strbuf *out, const char *prefix)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n-\tconst char *rel = path_relative(in, len, &sb, prefix, -1);\n+\tconst char *rel = relative_path(in, prefix, &sb);\n \tstrbuf_reset(out);\n \tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n \tstrbuf_release(&sb);\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221806","messageId":"c7b9df81fad6a5b87400ae7beb6e6c2b7d48a67b.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 04/16] Refactor quote_path_relative, remove unused params","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:28Z","receivedAt":"2013-06-24T15:21:28Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"After substitute path_relative() in quote.c with relative_path() from\npath.c, parameters (such as len and prefix_len) are obsolete in function\nquote_path_relative(). Remove unused parameters and change the order of\nparameters for quote_path_relative() function.\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 18 +++++++++---------\n builtin/grep.c | 5 ++---\n builtin/ls-files.c | 2 +-\n quote.c | 7 ++-----\n quote.h | 4 ++--\n wt-status.c | 17 ++++++++---------\n 6 files changed, 24 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 04e39..f77f95 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -56,7 +56,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \tif ((force_flag & REMOVE_DIR_KEEP_NESTED_GIT) &&\n \t\t\t!resolve_gitlink_ref(path->buf, \"HEAD\", submodule_head)) {\n \t\tif (!quiet) {\n-\t\t\tquote_path_relative(path->buf, strlen(path->buf), "ed, prefix);\n+\t\t\tquote_path_relative(path->buf, prefix, "ed);\n \t\t\tprintf(dry_run ? _(msg_would_skip_git_dir) : _(msg_skip_git_dir),\n \t\t\t\t\tquoted.buf);\n \t\t}\n@@ -70,7 +70,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t/* an empty dir could be removed even if it is unreadble */\n \t\tres = dry_run ? 0 : rmdir(path->buf);\n \t\tif (res) {\n-\t\t\tquote_path_relative(path->buf, strlen(path->buf), "ed, prefix);\n+\t\t\tquote_path_relative(path->buf, prefix, "ed);\n \t\t\twarning(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t*dir_gone = 0;\n \t\t}\n@@ -94,7 +94,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t\tif (remove_dirs(path, prefix, force_flag, dry_run, quiet, &gone))\n \t\t\t\tret = 1;\n \t\t\tif (gone) {\n-\t\t\t\tquote_path_relative(path->buf, strlen(path->buf), "ed, prefix);\n+\t\t\t\tquote_path_relative(path->buf, prefix, "ed);\n \t\t\t\tstring_list_append(&dels, quoted.buf);\n \t\t\t} else\n \t\t\t\t*dir_gone = 0;\n@@ -102,10 +102,10 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\t} else {\n \t\t\tres = dry_run ? 0 : unlink(path->buf);\n \t\t\tif (!res) {\n-\t\t\t\tquote_path_relative(path->buf, strlen(path->buf), "ed, prefix);\n+\t\t\t\tquote_path_relative(path->buf, prefix, "ed);\n \t\t\t\tstring_list_append(&dels, quoted.buf);\n \t\t\t} else {\n-\t\t\t\tquote_path_relative(path->buf, strlen(path->buf), "ed, prefix);\n+\t\t\t\tquote_path_relative(path->buf, prefix, "ed);\n \t\t\t\twarning(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t\t*dir_gone = 0;\n \t\t\t\tret = 1;\n@@ -127,7 +127,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \t\tif (!res)\n \t\t\t*dir_gone = 1;\n \t\telse {\n-\t\t\tquote_path_relative(path->buf, strlen(path->buf), "ed, prefix);\n+\t\t\tquote_path_relative(path->buf, prefix, "ed);\n \t\t\twarning(_(msg_warn_remove_failed), quoted.buf);\n \t\t\t*dir_gone = 0;\n \t\t\tret = 1;\n@@ -262,7 +262,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n \t\t\t\t\terrors++;\n \t\t\t\tif (gone && !quiet) {\n-\t\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n+\t\t\t\t\tqname = quote_path_relative(directory.buf, prefix, &buf);\n \t\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t\t}\n \t\t\t}\n@@ -272,11 +272,11 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue;\n \t\t\tres = dry_run ? 0 : unlink(ent->name);\n \t\t\tif (res) {\n-\t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n+\t\t\t\tqname = quote_path_relative(ent->name, prefix, &buf);\n \t\t\t\twarning(_(msg_warn_remove_failed), qname);\n \t\t\t\terrors++;\n \t\t\t} else if (!quiet) {\n-\t\t\t\tqname = quote_path_relative(ent->name, -1, &buf, prefix);\n+\t\t\t\tqname = quote_path_relative(ent->name, prefix, &buf);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t}\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 159e65..a419c 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -286,8 +286,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,\n \tstruct strbuf pathbuf = STRBUF_INIT;\n \n \tif (opt->relative && opt->prefix_length) {\n-\t\tquote_path_relative(filename + tree_name_len, -1, &pathbuf,\n-\t\t\t\t opt->prefix);\n+\t\tquote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);\n \t\tstrbuf_insert(&pathbuf, 0, filename, tree_name_len);\n \t} else {\n \t\tstrbuf_addstr(&pathbuf, filename);\n@@ -318,7 +317,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \tstruct strbuf buf = STRBUF_INIT;\n \n \tif (opt->relative && opt->prefix_length)\n-\t\tquote_path_relative(filename, -1, &buf, opt->prefix);\n+\t\tquote_path_relative(filename, opt->prefix, &buf);\n \telse\n \t\tstrbuf_addstr(&buf, filename);\n \ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 3a410c..16d4f 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -391,7 +391,7 @@ int report_path_error(const char *ps_matched, const char **pathspec, const char\n \t\tif (found_dup)\n \t\t\tcontinue;\n \n-\t\tname = quote_path_relative(pathspec[num], -1, &sb, prefix);\n+\t\tname = quote_path_relative(pathspec[num], prefix, &sb);\n \t\terror(\"pathspec '%s' did not match any file(s) known to git.\",\n \t\t name);\n \t\terrors++;\ndiff --git a/quote.c b/quote.c\nindex 64ff3..ebb8 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -325,8 +325,8 @@ void write_name_quoted_relative(const char *name, size_t len,\n }\n \n /* quote path as relative to the given prefix */\n-char *quote_path_relative(const char *in, int len,\n-\t\t\t struct strbuf *out, const char *prefix)\n+char *quote_path_relative(const char *in, const char *prefix,\n+\t\t\t struct strbuf *out)\n {\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *rel = relative_path(in, prefix, &sb);\n@@ -334,9 +334,6 @@ char *quote_path_relative(const char *in, int len,\n \tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n \tstrbuf_release(&sb);\n \n-\tif (!out->len)\n-\t\tstrbuf_addstr(out, \"./\");\n-\n \treturn out->buf;\n }\n \ndiff --git a/quote.h b/quote.h\nindex 13315..5610159 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -65,8 +65,8 @@ extern void write_name_quoted_relative(const char *name, size_t len,\n \t\tFILE *fp, int terminator);\n \n /* quote path as relative to the given prefix */\n-extern char *quote_path_relative(const char *in, int len,\n-\t\t\t struct strbuf *out, const char *prefix);\n+extern char *quote_path_relative(const char *in, const char *prefix,\n+\t\t\t struct strbuf *out);\n \n /* quoting as a string literal for other languages */\n extern void perl_quote_print(FILE *stream, const char *src);\ndiff --git a/wt-status.c b/wt-status.c\nindex 438a40..85580 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -243,7 +243,7 @@ static void wt_status_print_unmerged_data(struct wt_status *s,\n \tstruct strbuf onebuf = STRBUF_INIT;\n \tconst char *one, *how = _(\"bug\");\n \n-\tone = quote_path(it->string, -1, &onebuf, s->prefix);\n+\tone = quote_path(it->string, s->prefix, &onebuf);\n \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n \tswitch (d->stagemask) {\n \tcase 1: how = _(\"both deleted:\"); break;\n@@ -297,8 +297,8 @@ static void wt_status_print_change_data(struct wt_status *s,\n \t\t change_type);\n \t}\n \n-\tone = quote_path(one_name, -1, &onebuf, s->prefix);\n-\ttwo = quote_path(two_name, -1, &twobuf, s->prefix);\n+\tone = quote_path(one_name, s->prefix, &onebuf);\n+\ttwo = quote_path(two_name, s->prefix, &twobuf);\n \n \tstatus_printf(s, color(WT_STATUS_HEADER, s), \"\\t\");\n \tswitch (status) {\n@@ -706,8 +706,7 @@ static void wt_status_print_other(struct wt_status *s,\n \t\tstruct string_list_item *it;\n \t\tconst char *path;\n \t\tit = &(l->items[i]);\n-\t\tpath = quote_path(it->string, strlen(it->string),\n-\t\t\t\t &buf, s->prefix);\n+\t\tpath = quote_path(it->string, s->prefix, &buf);\n \t\tif (column_active(s->colopts)) {\n \t\t\tstring_list_append(&output, path);\n \t\t\tcontinue;\n@@ -1291,7 +1290,7 @@ static void wt_shortstatus_unmerged(struct string_list_item *it,\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tone = quote_path(it->string, -1, &onebuf, s->prefix);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf);\n \t\tprintf(\" %s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n \t}\n@@ -1319,7 +1318,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n \t\tif (d->head_path) {\n-\t\t\tone = quote_path(d->head_path, -1, &onebuf, s->prefix);\n+\t\t\tone = quote_path(d->head_path, s->prefix, &onebuf);\n \t\t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n \t\t\t\tputchar('\"');\n \t\t\t\tstrbuf_addch(&onebuf, '\"');\n@@ -1328,7 +1327,7 @@ static void wt_shortstatus_status(struct string_list_item *it,\n \t\t\tprintf(\"%s -> \", one);\n \t\t\tstrbuf_release(&onebuf);\n \t\t}\n-\t\tone = quote_path(it->string, -1, &onebuf, s->prefix);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf);\n \t\tif (*one != '\"' && strchr(one, ' ') != NULL) {\n \t\t\tputchar('\"');\n \t\t\tstrbuf_addch(&onebuf, '\"');\n@@ -1347,7 +1346,7 @@ static void wt_shortstatus_other(struct string_list_item *it,\n \t} else {\n \t\tstruct strbuf onebuf = STRBUF_INIT;\n \t\tconst char *one;\n-\t\tone = quote_path(it->string, -1, &onebuf, s->prefix);\n+\t\tone = quote_path(it->string, s->prefix, &onebuf);\n \t\tcolor_fprintf(s->fp, color(WT_STATUS_UNTRACKED, s), \"%s\", sign);\n \t\tprintf(\" %s\\n\", one);\n \t\tstrbuf_release(&onebuf);\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221807","messageId":"6e5f34b62ff6a7d6b379a5cabc07b849539fb08f.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 05/16] Refactor write_name_quoted_relative, remove unused params","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:29Z","receivedAt":"2013-06-24T15:21:29Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"After substitute path_relative() in quote.c with relative_path() from\npath.c, parameters (such as len and prefix_len) are obsolete in function\nwrite_name_quoted_relative(). Remove unused parameters from\nwrite_name_quoted_relative() and related functions.\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Junio C Hamano \n---\n builtin/ls-files.c | 14 ++++++++------\n quote.c | 3 +--\n quote.h | 3 +--\n 3 files changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 16d4f..df83f9 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -46,10 +46,12 @@ static const char *tag_modified = \"\";\n static const char *tag_skip_worktree = \"\";\n static const char *tag_resolve_undo = \"\";\n \n-static void write_name(const char* name, size_t len)\n+static void write_name(const char *name)\n {\n-\twrite_name_quoted_relative(name, len, prefix, prefix_len, stdout,\n-\t\t\tline_terminator);\n+\n+\t/* turn off prefix, if run with \"--full-name\" */\n+\twrite_name_quoted_relative(name, prefix_len ? prefix : NULL,\n+\t\t\t\t stdout, line_terminator);\n }\n \n static void show_dir_entry(const char *tag, struct dir_entry *ent)\n@@ -63,7 +65,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)\n \t\treturn;\n \n \tfputs(tag, stdout);\n-\twrite_name(ent->name, ent->len);\n+\twrite_name(ent->name);\n }\n \n static void show_other_files(struct dir_struct *dir)\n@@ -163,7 +165,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)\n \t\t find_unique_abbrev(ce->sha1,abbrev),\n \t\t ce_stage(ce));\n \t}\n-\twrite_name(ce->name, ce_namelen(ce));\n+\twrite_name(ce->name);\n \tif (debug_mode) {\n \t\tstruct stat_data *sd = &ce->ce_stat_data;\n \n@@ -198,7 +200,7 @@ static void show_ru_info(void)\n \t\t\tprintf(\"%s%06o %s %d\\t\", tag_resolve_undo, ui->mode[i],\n \t\t\t find_unique_abbrev(ui->sha1[i], abbrev),\n \t\t\t i + 1);\n-\t\t\twrite_name(path, len);\n+\t\t\twrite_name(path);\n \t\t}\n \t}\n }\ndiff --git a/quote.c b/quote.c\nindex ebb8..5c880 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -312,8 +312,7 @@ void write_name_quotedpfx(const char *pfx, size_t pfxlen,\n \tfputc(terminator, fp);\n }\n \n-void write_name_quoted_relative(const char *name, size_t len,\n-\t\t\t\tconst char *prefix, size_t prefix_len,\n+void write_name_quoted_relative(const char *name, const char *prefix,\n \t\t\t\tFILE *fp, int terminator)\n {\n \tstruct strbuf sb = STRBUF_INIT;\ndiff --git a/quote.h b/quote.h\nindex 5610159..ed110 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -60,8 +60,7 @@ extern void quote_two_c_style(struct strbuf *, const char *, const char *, int);\n extern void write_name_quoted(const char *name, FILE *, int terminator);\n extern void write_name_quotedpfx(const char *pfx, size_t pfxlen,\n const char *name, FILE *, int terminator);\n-extern void write_name_quoted_relative(const char *name, size_t len,\n-\t\tconst char *prefix, size_t prefix_len,\n+extern void write_name_quoted_relative(const char *name, const char *prefix,\n \t\tFILE *fp, int terminator);\n \n /* quote path as relative to the given prefix */\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221808","messageId":"37345475f7166717c7e3cbfab7e179605dafabdd.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 06/16] git-clean: refactor git-clean into two phases","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:30Z","receivedAt":"2013-06-24T15:21:30Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Before introducing interactive git-clean, refactor git-clean operations\ninto two phases:\n\n * hold cleaning items in del_list,\n * and remove them in a separate loop at the end.\n\nWe will introduce interactive git-clean between the two phases. The\ninteractive git-clean will show what would be done and must confirm\nbefore do real cleaning.\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 64 ++++++++++++++++++++++++++++++++++++++++-----------------\n 1 file changed, 45 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex f77f95..77ec1 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -15,6 +15,7 @@\n #include \"quote.h\"\n \n static int force = -1; /* unset */\n+static struct string_list del_list = STRING_LIST_INIT_DUP;\n \n static const char *const builtin_clean_usage[] = {\n \tN_(\"git clean [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n@@ -148,12 +149,13 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tint dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0;\n \tint ignored_only = 0, config_set = 0, errors = 0, gone = 1;\n \tint rm_flags = REMOVE_DIR_KEEP_NESTED_GIT;\n-\tstruct strbuf directory = STRBUF_INIT;\n+\tstruct strbuf abs_path = STRBUF_INIT;\n \tstruct dir_struct dir;\n \tstatic const char **pathspec;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct string_list exclude_list = STRING_LIST_INIT_NODUP;\n \tstruct exclude_list *el;\n+\tstruct string_list_item *item;\n \tconst char *qname;\n \tchar *seen = NULL;\n \tstruct option options[] = {\n@@ -223,6 +225,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\tint matches = 0;\n \t\tstruct cache_entry *ce;\n \t\tstruct stat st;\n+\t\tconst char *rel;\n \n \t\t/*\n \t\t * Remove the '/' at the end that directory\n@@ -242,13 +245,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue; /* Yup, this one exists unmerged */\n \t\t}\n \n-\t\t/*\n-\t\t * we might have removed this as part of earlier\n-\t\t * recursive directory removal, so lstat() here could\n-\t\t * fail with ENOENT.\n-\t\t */\n \t\tif (lstat(ent->name, &st))\n-\t\t\tcontinue;\n+\t\t\tdie_errno(\"Cannot lstat '%s'\", ent->name);\n \n \t\tif (pathspec) {\n \t\t\tmemset(seen, 0, argc > 0 ? argc : 1);\n@@ -257,33 +255,61 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t}\n \n \t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\tstrbuf_addstr(&directory, ent->name);\n \t\t\tif (remove_directories || (matches == MATCHED_EXACTLY)) {\n-\t\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n-\t\t\t\t\terrors++;\n-\t\t\t\tif (gone && !quiet) {\n-\t\t\t\t\tqname = quote_path_relative(directory.buf, prefix, &buf);\n-\t\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n-\t\t\t\t}\n+\t\t\t\trel = relative_path(ent->name, prefix, &buf);\n+\t\t\t\tstring_list_append(&del_list, rel);\n \t\t\t}\n-\t\t\tstrbuf_reset(&directory);\n \t\t} else {\n \t\t\tif (pathspec && !matches)\n \t\t\t\tcontinue;\n-\t\t\tres = dry_run ? 0 : unlink(ent->name);\n+\t\t\trel = relative_path(ent->name, prefix, &buf);\n+\t\t\tstring_list_append(&del_list, rel);\n+\t\t}\n+\t}\n+\n+\t/* TODO: do interactive git-clean here, which will modify del_list */\n+\n+\tfor_each_string_list_item(item, &del_list) {\n+\t\tstruct stat st;\n+\n+\t\tif (prefix)\n+\t\t\tstrbuf_addstr(&abs_path, prefix);\n+\n+\t\tstrbuf_addstr(&abs_path, item->string);\n+\n+\t\t/*\n+\t\t * we might have removed this as part of earlier\n+\t\t * recursive directory removal, so lstat() here could\n+\t\t * fail with ENOENT.\n+\t\t */\n+\t\tif (lstat(abs_path.buf, &st))\n+\t\t\tcontinue;\n+\n+\t\tif (S_ISDIR(st.st_mode)) {\n+\t\t\tif (remove_dirs(&abs_path, prefix, rm_flags, dry_run, quiet, &gone))\n+\t\t\t\terrors++;\n+\t\t\tif (gone && !quiet) {\n+\t\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n+\t\t\t}\n+\t\t} else {\n+\t\t\tres = dry_run ? 0 : unlink(abs_path.buf);\n \t\t\tif (res) {\n-\t\t\t\tqname = quote_path_relative(ent->name, prefix, &buf);\n+\t\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n \t\t\t\twarning(_(msg_warn_remove_failed), qname);\n \t\t\t\terrors++;\n \t\t\t} else if (!quiet) {\n-\t\t\t\tqname = quote_path_relative(ent->name, prefix, &buf);\n+\t\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n \t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n \t\t\t}\n \t\t}\n+\t\tstrbuf_reset(&abs_path);\n \t}\n \tfree(seen);\n \n-\tstrbuf_release(&directory);\n+\tstrbuf_release(&abs_path);\n+\tstrbuf_release(&buf);\n+\tstring_list_clear(&del_list, 0);\n \tstring_list_clear(&exclude_list, 0);\n \treturn (errors != 0);\n }\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221809","messageId":"e9b5bf5dad8b590df282e44c675025e11e2e3295.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 07/16] git-clean: add support for -i/--interactive","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:31Z","receivedAt":"2013-06-24T15:21:31Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Show what would be done and the user must confirm before actually\ncleaning.\n\n Would remove ...\n Would remove ...\n Would remove ...\n\n Remove [y/n]?\n\nPress \"y\" to start cleaning, and press \"n\" if you want to abort.\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-clean.txt | 10 ++++++--\n builtin/clean.c | 57 +++++++++++++++++++++++++++++++++++++++++----\n 2 files changed, 60 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt\nindex bdc3a..186e34 100644\n--- a/Documentation/git-clean.txt\n+++ b/Documentation/git-clean.txt\n@@ -8,7 +8,7 @@ git-clean - Remove untracked files from the working tree\n SYNOPSIS\n --------\n [verse]\n-'git clean' [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...\n+'git clean' [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...\n \n DESCRIPTION\n -----------\n@@ -34,7 +34,13 @@ OPTIONS\n -f::\n --force::\n \tIf the Git configuration variable clean.requireForce is not set\n-\tto false, 'git clean' will refuse to run unless given -f or -n.\n+\tto false, 'git clean' will refuse to run unless given -f, -n or\n+\t-i.\n+\n+-i::\n+--interactive::\n+\tShow what would be done and the user must confirm before actually\n+\tcleaning.\n \n -n::\n --dry-run::\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 77ec1..698fb 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -15,10 +15,11 @@\n #include \"quote.h\"\n \n static int force = -1; /* unset */\n+static int interactive;\n static struct string_list del_list = STRING_LIST_INIT_DUP;\n \n static const char *const builtin_clean_usage[] = {\n-\tN_(\"git clean [-d] [-f] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n+\tN_(\"git clean [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n \tNULL\n };\n \n@@ -143,6 +144,50 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \treturn ret;\n }\n \n+static void interactive_main_loop(void)\n+{\n+\tstruct strbuf confirm = STRBUF_INIT;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct string_list_item *item;\n+\tconst char *qname;\n+\n+\twhile (del_list.nr) {\n+\t\tputchar('\\n');\n+\t\tfor_each_string_list_item(item, &del_list) {\n+\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\tprintf(_(msg_would_remove), qname);\n+\t\t}\n+\t\tputchar('\\n');\n+\n+\t\tprintf(_(\"Remove [y/n]? \"));\n+\t\tif (strbuf_getline(&confirm, stdin, '\\n') != EOF) {\n+\t\t\tstrbuf_trim(&confirm);\n+\t\t} else {\n+\t\t\t/* Ctrl-D is the same as \"quit\" */\n+\t\t\tstring_list_clear(&del_list, 0);\n+\t\t\tputchar('\\n');\n+\t\t\tprintf_ln(\"Bye.\");\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tif (confirm.len) {\n+\t\t\tif (!strncasecmp(confirm.buf, \"yes\", confirm.len)) {\n+\t\t\t\tbreak;\n+\t\t\t} else if (!strncasecmp(confirm.buf, \"no\", confirm.len) ||\n+\t\t\t\t !strncasecmp(confirm.buf, \"quit\", confirm.len)) {\n+\t\t\t\tstring_list_clear(&del_list, 0);\n+\t\t\t\tprintf_ln(\"Bye.\");\n+\t\t\t\tbreak;\n+\t\t\t} else {\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&confirm);\n+}\n+\n int cmd_clean(int argc, const char **argv, const char *prefix)\n {\n \tint i, res;\n@@ -162,6 +207,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\tOPT__QUIET(&quiet, N_(\"do not print names of files removed\")),\n \t\tOPT__DRY_RUN(&dry_run, N_(\"dry run\")),\n \t\tOPT__FORCE(&force, N_(\"force\")),\n+\t\tOPT_BOOL('i', \"interactive\", &interactive, N_(\"interactive cleaning\")),\n \t\tOPT_BOOLEAN('d', NULL, &remove_directories,\n \t\t\t\tN_(\"remove whole directories\")),\n \t\t{ OPTION_CALLBACK, 'e', \"exclude\", &exclude_list, N_(\"pattern\"),\n@@ -188,12 +234,12 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tif (ignored && ignored_only)\n \t\tdie(_(\"-x and -X cannot be used together\"));\n \n-\tif (!dry_run && !force) {\n+\tif (!interactive && !dry_run && !force) {\n \t\tif (config_set)\n-\t\t\tdie(_(\"clean.requireForce set to true and neither -n nor -f given; \"\n+\t\t\tdie(_(\"clean.requireForce set to true and neither -i, -n nor -f given; \"\n \t\t\t\t \"refusing to clean\"));\n \t\telse\n-\t\t\tdie(_(\"clean.requireForce defaults to true and neither -n nor -f given; \"\n+\t\t\tdie(_(\"clean.requireForce defaults to true and neither -i, -n nor -f given; \"\n \t\t\t\t \"refusing to clean\"));\n \t}\n \n@@ -267,7 +313,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \t\t}\n \t}\n \n-\t/* TODO: do interactive git-clean here, which will modify del_list */\n+\tif (interactive && del_list.nr > 0)\n+\t\tinteractive_main_loop();\n \n \tfor_each_string_list_item(item, &del_list) {\n \t\tstruct stat st;\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221810","messageId":"019a43b98c74c3fa42e8c34ed3d2fb295e5a312b.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 08/16] git-clean: show items of del_list in columns","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:32Z","receivedAt":"2013-06-24T15:21:32Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"When there are lots of items to be cleaned, it is hard to see them all\nin one screen. Show them in columns will solve this problem.\n\nSigned-off-by: Jiang Xin \nComments-by: Matthieu Moy \nSigned-off-by: Junio C Hamano \n---\n Documentation/config.txt | 4 ++++\n builtin/clean.c | 49 +++++++++++++++++++++++++++++++++++++++---------\n 2 files changed, 44 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex e203..c415f 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -959,6 +959,10 @@ column.branch::\n \tSpecify whether to output branch listing in `git branch` in columns.\n \tSee `column.ui` for details.\n \n+column.clean::\n+\tSpecify the layout when list items in `git clean -i`, which always\n+\tshows files and directories in columns. See `column.ui` for details.\n+\n column.status::\n \tSpecify whether to output untracked files in `git status` in columns.\n \tSee `column.ui` for details.\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 698fb..75cc6 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -13,10 +13,12 @@\n #include \"refs.h\"\n #include \"string-list.h\"\n #include \"quote.h\"\n+#include \"column.h\"\n \n static int force = -1; /* unset */\n static int interactive;\n static struct string_list del_list = STRING_LIST_INIT_DUP;\n+static unsigned int colopts;\n \n static const char *const builtin_clean_usage[] = {\n \tN_(\"git clean [-d] [-f] [-i] [-n] [-q] [-e ] [-x | -X] [--] ...\"),\n@@ -31,8 +33,13 @@ static const char *msg_warn_remove_failed = N_(\"failed to remove %s\");\n \n static int git_clean_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(var, \"clean.requireforce\"))\n+\tif (!prefixcmp(var, \"column.\"))\n+\t\treturn git_column_config(var, value, \"clean\", &colopts);\n+\n+\tif (!strcmp(var, \"clean.requireforce\")) {\n \t\tforce = !git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n \treturn git_default_config(var, value, cb);\n }\n \n@@ -144,21 +151,46 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,\n \treturn ret;\n }\n \n-static void interactive_main_loop(void)\n+static void pretty_print_dels(void)\n {\n-\tstruct strbuf confirm = STRBUF_INIT;\n-\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct string_list list = STRING_LIST_INIT_DUP;\n \tstruct string_list_item *item;\n+\tstruct strbuf buf = STRBUF_INIT;\n \tconst char *qname;\n+\tstruct column_options copts;\n+\n+\tfor_each_string_list_item(item, &del_list) {\n+\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\tstring_list_append(&list, qname);\n+\t}\n+\n+\t/*\n+\t * always enable column display, we only consult column.*\n+\t * about layout strategy and stuff\n+\t */\n+\tcolopts = (colopts & ~COL_ENABLE_MASK) | COL_ENABLED;\n+\tmemset(&copts, 0, sizeof(copts));\n+\tcopts.indent = \" \";\n+\tcopts.padding = 2;\n+\tprint_columns(&list, colopts, &copts);\n+\tputchar('\\n');\n+\tstrbuf_release(&buf);\n+\tstring_list_clear(&list, 0);\n+}\n+\n+static void interactive_main_loop(void)\n+{\n+\tstruct strbuf confirm = STRBUF_INIT;\n \n \twhile (del_list.nr) {\n \t\tputchar('\\n');\n-\t\tfor_each_string_list_item(item, &del_list) {\n-\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n-\t\t\tprintf(_(msg_would_remove), qname);\n-\t\t}\n+\t\tprintf_ln(Q_(\"Would remove the following item:\",\n+\t\t\t \"Would remove the following items:\",\n+\t\t\t del_list.nr));\n \t\tputchar('\\n');\n \n+\t\tpretty_print_dels();\n+\n \t\tprintf(_(\"Remove [y/n]? \"));\n \t\tif (strbuf_getline(&confirm, stdin, '\\n') != EOF) {\n \t\t\tstrbuf_trim(&confirm);\n@@ -184,7 +216,6 @@ static void interactive_main_loop(void)\n \t\t}\n \t}\n \n-\tstrbuf_release(&buf);\n \tstrbuf_release(&confirm);\n }\n \n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221811","messageId":"9fd387b5ab08a13d37025c4af05639d278520eed.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 09/16] git-clean: add colors to interactive git-clean","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:33Z","receivedAt":"2013-06-24T15:21:33Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Show header, help, error messages, and prompt in colors for interactive\ngit-clean. Re-use config variables, such as \"color.interactive\" and\n\"color.interactive.\" for command `git-add--interactive`.\n\nSigned-off-by: Jiang Xin \nComments-by: Matthieu Moy \nSigned-off-by: Junio C Hamano \n---\n Documentation/config.txt | 17 +++++------\n builtin/clean.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 81 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c415f..1b31f 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -879,16 +879,17 @@ The values of these variables may be specified as in color.branch..\n \n color.interactive::\n \tWhen set to `always`, always use colors for interactive prompts\n-\tand displays (such as those used by \"git-add --interactive\").\n-\tWhen false (or `never`), never. When set to `true` or `auto`, use\n-\tcolors only when the output is to the terminal. Defaults to false.\n+\tand displays (such as those used by \"git-add --interactive\" and\n+\t\"git-clean --interactive\"). When false (or `never`), never.\n+\tWhen set to `true` or `auto`, use colors only when the output is\n+\tto the terminal. Defaults to false.\n \n color.interactive.::\n-\tUse customized color for 'git add --interactive'\n-\toutput. `` may be `prompt`, `header`, `help` or `error`, for\n-\tfour distinct types of normal output from interactive\n-\tcommands. The values of these variables may be specified as\n-\tin color.branch..\n+\tUse customized color for 'git add --interactive' and 'git clean\n+\t--interactive' output. `` may be `prompt`, `header`, `help`\n+\tor `error`, for four distinct types of normal output from\n+\tinteractive commands. The values of these variables may be\n+\tspecified as in color.branch..\n \n color.pager::\n \tA boolean to enable/disable colored output when the pager is in\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 75cc6..dfa99b 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -14,6 +14,7 @@\n #include \"string-list.h\"\n #include \"quote.h\"\n #include \"column.h\"\n+#include \"color.h\"\n \n static int force = -1; /* unset */\n static int interactive;\n@@ -31,16 +32,82 @@ static const char *msg_skip_git_dir = N_(\"Skipping repository %s\\n\");\n static const char *msg_would_skip_git_dir = N_(\"Would skip repository %s\\n\");\n static const char *msg_warn_remove_failed = N_(\"failed to remove %s\");\n \n+static int clean_use_color = -1;\n+static char clean_colors[][COLOR_MAXLEN] = {\n+\tGIT_COLOR_RESET,\n+\tGIT_COLOR_NORMAL,\t/* PLAIN */\n+\tGIT_COLOR_BOLD_BLUE,\t/* PROMPT */\n+\tGIT_COLOR_BOLD,\t\t/* HEADER */\n+\tGIT_COLOR_BOLD_RED,\t/* HELP */\n+\tGIT_COLOR_BOLD_RED,\t/* ERROR */\n+};\n+enum color_clean {\n+\tCLEAN_COLOR_RESET = 0,\n+\tCLEAN_COLOR_PLAIN = 1,\n+\tCLEAN_COLOR_PROMPT = 2,\n+\tCLEAN_COLOR_HEADER = 3,\n+\tCLEAN_COLOR_HELP = 4,\n+\tCLEAN_COLOR_ERROR = 5,\n+};\n+\n+static int parse_clean_color_slot(const char *var)\n+{\n+\tif (!strcasecmp(var, \"reset\"))\n+\t\treturn CLEAN_COLOR_RESET;\n+\tif (!strcasecmp(var, \"plain\"))\n+\t\treturn CLEAN_COLOR_PLAIN;\n+\tif (!strcasecmp(var, \"prompt\"))\n+\t\treturn CLEAN_COLOR_PROMPT;\n+\tif (!strcasecmp(var, \"header\"))\n+\t\treturn CLEAN_COLOR_HEADER;\n+\tif (!strcasecmp(var, \"help\"))\n+\t\treturn CLEAN_COLOR_HELP;\n+\tif (!strcasecmp(var, \"error\"))\n+\t\treturn CLEAN_COLOR_ERROR;\n+\treturn -1;\n+}\n+\n static int git_clean_config(const char *var, const char *value, void *cb)\n {\n \tif (!prefixcmp(var, \"column.\"))\n \t\treturn git_column_config(var, value, \"clean\", &colopts);\n \n+\t/* honors the color.interactive* config variables which also\n+\t applied in git-add--interactive and git-stash */\n+\tif (!strcmp(var, \"color.interactive\")) {\n+\t\tclean_use_color = git_config_colorbool(var, value);\n+\t\treturn 0;\n+\t}\n+\tif (!prefixcmp(var, \"color.interactive.\")) {\n+\t\tint slot = parse_clean_color_slot(var +\n+\t\t\t\t\t\t strlen(\"color.interactive.\"));\n+\t\tif (slot < 0)\n+\t\t\treturn 0;\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tcolor_parse(value, var, clean_colors[slot]);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"clean.requireforce\")) {\n \t\tforce = !git_config_bool(var, value);\n \t\treturn 0;\n \t}\n-\treturn git_default_config(var, value, cb);\n+\n+\t/* inspect the color.ui config variable and others */\n+\treturn git_color_default_config(var, value, cb);\n+}\n+\n+static const char *clean_get_color(enum color_clean ix)\n+{\n+\tif (want_color(clean_use_color))\n+\t\treturn clean_colors[ix];\n+\treturn \"\";\n+}\n+\n+static void clean_print_color(enum color_clean ix)\n+{\n+\tprintf(\"%s\", clean_get_color(ix));\n }\n \n static int exclude_cb(const struct option *opt, const char *arg, int unset)\n@@ -184,14 +251,18 @@ static void interactive_main_loop(void)\n \n \twhile (del_list.nr) {\n \t\tputchar('\\n');\n+\t\tclean_print_color(CLEAN_COLOR_HEADER);\n \t\tprintf_ln(Q_(\"Would remove the following item:\",\n \t\t\t \"Would remove the following items:\",\n \t\t\t del_list.nr));\n+\t\tclean_print_color(CLEAN_COLOR_RESET);\n \t\tputchar('\\n');\n \n \t\tpretty_print_dels();\n \n+\t\tclean_print_color(CLEAN_COLOR_PROMPT);\n \t\tprintf(_(\"Remove [y/n]? \"));\n+\t\tclean_print_color(CLEAN_COLOR_RESET);\n \t\tif (strbuf_getline(&confirm, stdin, '\\n') != EOF) {\n \t\t\tstrbuf_trim(&confirm);\n \t\t} else {\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221812","messageId":"2f2952d5c4fd63811e51be30e69de92e476bb5b2.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 10/16] git-clean: use a git-add-interactive compatible UI","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:34Z","receivedAt":"2013-06-24T15:21:34Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Rewrite menu using a new method `list_and_choose`, which is borrowed\nfrom `git-add--interactive.perl`. We will use this framework to add\nnew actions for interactive git-clean later.\n\nPlease NOTE:\n\n * Method `list_and_choose` return an array of integers, and\n * it is up to you to free the allocated memory of the array.\n * The array ends with EOF.\n * If user pressed CTRL-D (i.e. EOF), no selection returned.\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 456 ++++++++++++++++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 427 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex dfa99b..df887 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -50,6 +50,36 @@ enum color_clean {\n \tCLEAN_COLOR_ERROR = 5,\n };\n \n+#define MENU_OPTS_SINGLETON\t\t01\n+#define MENU_OPTS_IMMEDIATE\t\t02\n+#define MENU_OPTS_LIST_ONLY\t\t04\n+\n+struct menu_opts {\n+\tconst char *header;\n+\tconst char *prompt;\n+\tint flags;\n+};\n+\n+#define MENU_RETURN_NO_LOOP\t\t10\n+\n+struct menu_item {\n+\tchar hotkey;\n+\tconst char *title;\n+\tint selected;\n+\tint (*fn)();\n+};\n+\n+enum menu_stuff_type {\n+\tMENU_STUFF_TYPE_STRING_LIST = 1,\n+\tMENU_STUFF_TYPE_MENU_ITEM\n+};\n+\n+struct menu_stuff {\n+\tenum menu_stuff_type type;\n+\tint nr;\n+\tvoid *stuff;\n+};\n+\n static int parse_clean_color_slot(const char *var)\n {\n \tif (!strcasecmp(var, \"reset\"))\n@@ -240,54 +270,422 @@ static void pretty_print_dels(void)\n \tcopts.indent = \" \";\n \tcopts.padding = 2;\n \tprint_columns(&list, colopts, &copts);\n-\tputchar('\\n');\n \tstrbuf_release(&buf);\n \tstring_list_clear(&list, 0);\n }\n \n-static void interactive_main_loop(void)\n+static void pretty_print_menus(struct string_list *menu_list)\n+{\n+\tunsigned int local_colopts = 0;\n+\tstruct column_options copts;\n+\n+\tlocal_colopts = COL_ENABLED | COL_ROW;\n+\tmemset(&copts, 0, sizeof(copts));\n+\tcopts.indent = \" \";\n+\tcopts.padding = 2;\n+\tprint_columns(menu_list, local_colopts, &copts);\n+}\n+\n+static void prompt_help_cmd(int singleton)\n+{\n+\tclean_print_color(CLEAN_COLOR_HELP);\n+\tprintf_ln(singleton ?\n+\t\t _(\"Prompt help:\\n\"\n+\t\t \"1 - select a numbered item\\n\"\n+\t\t \"foo - select item based on unique prefix\\n\"\n+\t\t \" - (empty) select nothing\") :\n+\t\t _(\"Prompt help:\\n\"\n+\t\t \"1 - select a single item\\n\"\n+\t\t \"3-5 - select a range of items\\n\"\n+\t\t \"2-3,6-9 - select multiple ranges\\n\"\n+\t\t \"foo - select item based on unique prefix\\n\"\n+\t\t \"-... - unselect specified items\\n\"\n+\t\t \"* - choose all items\\n\"\n+\t\t \" - (empty) finish selecting\"));\n+\tclean_print_color(CLEAN_COLOR_RESET);\n+}\n+\n+/*\n+ * display menu stuff with number prefix and hotkey highlight\n+ */\n+static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen)\n+{\n+\tstruct string_list menu_list = STRING_LIST_INIT_DUP;\n+\tstruct strbuf menu = STRBUF_INIT;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct menu_item *menu_item;\n+\tstruct string_list_item *string_list_item;\n+\tint i;\n+\n+\tswitch (stuff->type) {\n+\tdefault:\n+\t\tdie(\"Bad type of menu_staff when print menu\");\n+\tcase MENU_STUFF_TYPE_MENU_ITEM:\n+\t\tmenu_item = (struct menu_item *)stuff->stuff;\n+\t\tfor (i = 0; i < stuff->nr; i++, menu_item++) {\n+\t\t\tconst char *p;\n+\t\t\tint highlighted = 0;\n+\n+\t\t\tp = menu_item->title;\n+\t\t\tif ((*chosen)[i] < 0)\n+\t\t\t\t(*chosen)[i] = menu_item->selected ? 1 : 0;\n+\t\t\tstrbuf_addf(&menu, \"%s%2d: \", (*chosen)[i] ? \"*\" : \" \", i+1);\n+\t\t\tfor (; *p; p++) {\n+\t\t\t\tif (!highlighted && *p == menu_item->hotkey) {\n+\t\t\t\t\tstrbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_PROMPT));\n+\t\t\t\t\tstrbuf_addch(&menu, *p);\n+\t\t\t\t\tstrbuf_addstr(&menu, clean_get_color(CLEAN_COLOR_RESET));\n+\t\t\t\t\thighlighted = 1;\n+\t\t\t\t} else {\n+\t\t\t\t\tstrbuf_addch(&menu, *p);\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tstring_list_append(&menu_list, menu.buf);\n+\t\t\tstrbuf_reset(&menu);\n+\t\t}\n+\t\tbreak;\n+\tcase MENU_STUFF_TYPE_STRING_LIST:\n+\t\ti = 0;\n+\t\tfor_each_string_list_item(string_list_item, (struct string_list *)stuff->stuff) {\n+\t\t\tif ((*chosen)[i] < 0)\n+\t\t\t\t(*chosen)[i] = 0;\n+\t\t\tstrbuf_addf(&menu, \"%s%2d: %s\",\n+\t\t\t\t (*chosen)[i] ? \"*\" : \" \", i+1, string_list_item->string);\n+\t\t\tstring_list_append(&menu_list, menu.buf);\n+\t\t\tstrbuf_reset(&menu);\n+\t\t\ti++;\n+\t\t}\n+\t\tbreak;\n+\t}\n+\n+\tpretty_print_menus(&menu_list);\n+\n+\tstrbuf_release(&menu);\n+\tstrbuf_release(&buf);\n+\tstring_list_clear(&menu_list, 0);\n+}\n+\n+/*\n+ * Parse user input, and return choice(s) for menu (menu_stuff).\n+ *\n+ * Input\n+ * (for single choice)\n+ * 1 - select a numbered item\n+ * foo - select item based on menu title\n+ * - (empty) select nothing\n+ *\n+ * (for multiple choice)\n+ * 1 - select a single item\n+ * 3-5 - select a range of items\n+ * 2-3,6-9 - select multiple ranges\n+ * foo - select item based on menu title\n+ * -... - unselect specified items\n+ * * - choose all items\n+ * - (empty) finish selecting\n+ *\n+ * The parse result will be saved in array **chosen, and\n+ * return number of total selections.\n+ */\n+static int parse_choice(struct menu_stuff *menu_stuff,\n+\t\t\tint is_single,\n+\t\t\tstruct strbuf input,\n+\t\t\tint **chosen)\n+{\n+\tstruct strbuf **choice_list, **ptr;\n+\tstruct menu_item *menu_item;\n+\tstruct string_list_item *string_list_item;\n+\tint nr = 0;\n+\tint i;\n+\n+\tif (is_single) {\n+\t\tchoice_list = strbuf_split_max(&input, '\\n', 0);\n+\t} else {\n+\t\tchar *p = input.buf;\n+\t\tdo {\n+\t\t\tif (*p == ',')\n+\t\t\t\t*p = ' ';\n+\t\t} while (*p++);\n+\t\tchoice_list = strbuf_split_max(&input, ' ', 0);\n+\t}\n+\n+\tfor (ptr = choice_list; *ptr; ptr++) {\n+\t\tchar *p;\n+\t\tint choose = 1;\n+\t\tint bottom = 0, top = 0;\n+\t\tint is_range, is_number;\n+\n+\t\tstrbuf_trim(*ptr);\n+\t\tif (!(*ptr)->len)\n+\t\t\tcontinue;\n+\n+\t\t/* Input that begins with '-'; unchoose */\n+\t\tif (*(*ptr)->buf == '-') {\n+\t\t\tchoose = 0;\n+\t\t\tstrbuf_remove((*ptr), 0, 1);\n+\t\t}\n+\n+\t\tis_range = 0;\n+\t\tis_number = 1;\n+\t\tfor (p = (*ptr)->buf; *p; p++) {\n+\t\t\tif ('-' == *p) {\n+\t\t\t\tif (!is_range) {\n+\t\t\t\t\tis_range = 1;\n+\t\t\t\t\tis_number = 0;\n+\t\t\t\t} else {\n+\t\t\t\t\tis_number = 0;\n+\t\t\t\t\tis_range = 0;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t} else if (!isdigit(*p)) {\n+\t\t\t\tis_number = 0;\n+\t\t\t\tis_range = 0;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (is_number) {\n+\t\t\tbottom = atoi((*ptr)->buf);\n+\t\t\ttop = bottom;\n+\t\t} else if (is_range) {\n+\t\t\tbottom = atoi((*ptr)->buf);\n+\t\t\t/* a range can be specified like 5-7 or 5- */\n+\t\t\tif (!*(strchr((*ptr)->buf, '-') + 1))\n+\t\t\t\ttop = menu_stuff->nr;\n+\t\t\telse\n+\t\t\t\ttop = atoi(strchr((*ptr)->buf, '-') + 1);\n+\t\t} else if (!strcmp((*ptr)->buf, \"*\")) {\n+\t\t\tbottom = 1;\n+\t\t\ttop = menu_stuff->nr;\n+\t\t} else {\n+\t\t\tswitch (menu_stuff->type) {\n+\t\t\tdefault:\n+\t\t\t\tdie(\"Bad type of menu_stuff when parse choice\");\n+\t\t\tcase MENU_STUFF_TYPE_MENU_ITEM:\n+\t\t\t\tmenu_item = (struct menu_item *)menu_stuff->stuff;\n+\t\t\t\tfor (i = 0; i < menu_stuff->nr; i++, menu_item++) {\n+\t\t\t\t\tif (((*ptr)->len == 1 &&\n+\t\t\t\t\t *(*ptr)->buf == menu_item->hotkey) ||\n+\t\t\t\t\t !strcasecmp((*ptr)->buf, menu_item->title)) {\n+\t\t\t\t\t\tbottom = i + 1;\n+\t\t\t\t\t\ttop = bottom;\n+\t\t\t\t\t\tbreak;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\tbreak;\n+\t\t\tcase MENU_STUFF_TYPE_STRING_LIST:\n+\t\t\t\tstring_list_item = ((struct string_list *)menu_stuff->stuff)->items;\n+\t\t\t\tfor (i = 0; i < menu_stuff->nr; i++, string_list_item++) {\n+\t\t\t\t\tif (!strcasecmp((*ptr)->buf, string_list_item->string)) {\n+\t\t\t\t\t\tbottom = i + 1;\n+\t\t\t\t\t\ttop = bottom;\n+\t\t\t\t\t\tbreak;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (top <= 0 || bottom <= 0 || top > menu_stuff->nr || bottom > top ||\n+\t\t (is_single && bottom != top)) {\n+\t\t\tclean_print_color(CLEAN_COLOR_ERROR);\n+\t\t\tprintf_ln(_(\"Huh (%s)?\"), (*ptr)->buf);\n+\t\t\tclean_print_color(CLEAN_COLOR_RESET);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tfor (i = bottom; i <= top; i++)\n+\t\t\t(*chosen)[i-1] = choose;\n+\t}\n+\n+\tstrbuf_list_free(choice_list);\n+\n+\tfor (i = 0; i < menu_stuff->nr; i++)\n+\t\tnr += (*chosen)[i];\n+\treturn nr;\n+}\n+\n+/*\n+ * Implement a git-add-interactive compatible UI, which is borrowed\n+ * from git-add--interactive.perl.\n+ *\n+ * Return value:\n+ *\n+ * - Return an array of integers\n+ * - , and it is up to you to free the allocated memory.\n+ * - The array ends with EOF.\n+ * - If user pressed CTRL-D (i.e. EOF), no selection returned.\n+ */\n+static int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff)\n+{\n+\tstruct strbuf choice = STRBUF_INIT;\n+\tint *chosen, *result;\n+\tint nr = 0;\n+\tint eof = 0;\n+\tint i;\n+\n+\tchosen = xmalloc(sizeof(int) * stuff->nr);\n+\t/* set chosen as uninitialized */\n+\tfor (i = 0; i < stuff->nr; i++)\n+\t\tchosen[i] = -1;\n+\n+\tfor (;;) {\n+\t\tif (opts->header) {\n+\t\t\tprintf_ln(\"%s%s%s\",\n+\t\t\t\t clean_get_color(CLEAN_COLOR_HEADER),\n+\t\t\t\t _(opts->header),\n+\t\t\t\t clean_get_color(CLEAN_COLOR_RESET));\n+\t\t}\n+\n+\t\t/* chosen will be initialized by print_highlight_menu_stuff */\n+\t\tprint_highlight_menu_stuff(stuff, &chosen);\n+\n+\t\tif (opts->flags & MENU_OPTS_LIST_ONLY)\n+\t\t\tbreak;\n+\n+\t\tif (opts->prompt) {\n+\t\t\tprintf(\"%s%s%s%s\",\n+\t\t\t clean_get_color(CLEAN_COLOR_PROMPT),\n+\t\t\t _(opts->prompt),\n+\t\t\t opts->flags & MENU_OPTS_SINGLETON ? \"> \" : \">> \",\n+\t\t\t clean_get_color(CLEAN_COLOR_RESET));\n+\t\t}\n+\n+\t\tif (strbuf_getline(&choice, stdin, '\\n') != EOF) {\n+\t\t\tstrbuf_trim(&choice);\n+\t\t} else {\n+\t\t\teof = 1;\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\t/* help for prompt */\n+\t\tif (!strcmp(choice.buf, \"?\")) {\n+\t\t\tprompt_help_cmd(opts->flags & MENU_OPTS_SINGLETON);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/* for a multiple-choice menu, press ENTER (empty) will return back */\n+\t\tif (!(opts->flags & MENU_OPTS_SINGLETON) && !choice.len)\n+\t\t\tbreak;\n+\n+\t\tnr = parse_choice(stuff,\n+\t\t\t\t opts->flags & MENU_OPTS_SINGLETON,\n+\t\t\t\t choice,\n+\t\t\t\t &chosen);\n+\n+\t\tif (opts->flags & MENU_OPTS_SINGLETON) {\n+\t\t\tif (nr)\n+\t\t\t\tbreak;\n+\t\t} else if (opts->flags & MENU_OPTS_IMMEDIATE) {\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n+\tif (eof) {\n+\t\tresult = xmalloc(sizeof(int));\n+\t\t*result = EOF;\n+\t} else {\n+\t\tint j = 0;\n+\n+\t\t/*\n+\t\t * recalculate nr, if return back from menu directly with\n+\t\t * default selections.\n+\t\t */\n+\t\tif (!nr) {\n+\t\t\tfor (i = 0; i < stuff->nr; i++)\n+\t\t\t\tnr += chosen[i];\n+\t\t}\n+\n+\t\tresult = xmalloc(sizeof(int) * (nr + 1));\n+\t\tmemset(result, 0, sizeof(int) * (nr + 1));\n+\t\tfor (i = 0; i < stuff->nr && j < nr; i++) {\n+\t\t\tif (chosen[i])\n+\t\t\t\tresult[j++] = i;\n+\t\t}\n+\t\tresult[j] = EOF;\n+\t}\n+\n+\tfree(chosen);\n+\tstrbuf_release(&choice);\n+\treturn result;\n+}\n+\n+static int clean_cmd(void)\n {\n-\tstruct strbuf confirm = STRBUF_INIT;\n+\treturn MENU_RETURN_NO_LOOP;\n+}\n+\n+static int quit_cmd(void)\n+{\n+\tstring_list_clear(&del_list, 0);\n+\tprintf_ln(_(\"Bye.\"));\n+\treturn MENU_RETURN_NO_LOOP;\n+}\n+\n+static int help_cmd(void)\n+{\n+\tclean_print_color(CLEAN_COLOR_HELP);\n+\tprintf_ln(_(\n+\t\t \"clean - start cleaning\\n\"\n+\t\t \"quit - stop cleaning\\n\"\n+\t\t \"help - this screen\\n\"\n+\t\t \"? - help for prompt selection\"\n+\t\t ));\n+\tclean_print_color(CLEAN_COLOR_RESET);\n+\treturn 0;\n+}\n \n+static void interactive_main_loop(void)\n+{\n \twhile (del_list.nr) {\n-\t\tputchar('\\n');\n+\t\tstruct menu_opts menu_opts;\n+\t\tstruct menu_stuff menu_stuff;\n+\t\tstruct menu_item menus[] = {\n+\t\t\t{'c', \"clean\",\t\t\t0, clean_cmd},\n+\t\t\t{'q', \"quit\",\t\t\t0, quit_cmd},\n+\t\t\t{'h', \"help\",\t\t\t0, help_cmd},\n+\t\t};\n+\t\tint *chosen;\n+\n+\t\tmenu_opts.header = N_(\"*** Commands ***\");\n+\t\tmenu_opts.prompt = N_(\"What now\");\n+\t\tmenu_opts.flags = MENU_OPTS_SINGLETON;\n+\n+\t\tmenu_stuff.type = MENU_STUFF_TYPE_MENU_ITEM;\n+\t\tmenu_stuff.stuff = menus;\n+\t\tmenu_stuff.nr = sizeof(menus) / sizeof(struct menu_item);\n+\n \t\tclean_print_color(CLEAN_COLOR_HEADER);\n \t\tprintf_ln(Q_(\"Would remove the following item:\",\n \t\t\t \"Would remove the following items:\",\n \t\t\t del_list.nr));\n \t\tclean_print_color(CLEAN_COLOR_RESET);\n-\t\tputchar('\\n');\n \n \t\tpretty_print_dels();\n \n-\t\tclean_print_color(CLEAN_COLOR_PROMPT);\n-\t\tprintf(_(\"Remove [y/n]? \"));\n-\t\tclean_print_color(CLEAN_COLOR_RESET);\n-\t\tif (strbuf_getline(&confirm, stdin, '\\n') != EOF) {\n-\t\t\tstrbuf_trim(&confirm);\n-\t\t} else {\n-\t\t\t/* Ctrl-D is the same as \"quit\" */\n-\t\t\tstring_list_clear(&del_list, 0);\n-\t\t\tputchar('\\n');\n-\t\t\tprintf_ln(\"Bye.\");\n-\t\t\tbreak;\n-\t\t}\n-\n-\t\tif (confirm.len) {\n-\t\t\tif (!strncasecmp(confirm.buf, \"yes\", confirm.len)) {\n-\t\t\t\tbreak;\n-\t\t\t} else if (!strncasecmp(confirm.buf, \"no\", confirm.len) ||\n-\t\t\t\t !strncasecmp(confirm.buf, \"quit\", confirm.len)) {\n-\t\t\t\tstring_list_clear(&del_list, 0);\n-\t\t\t\tprintf_ln(\"Bye.\");\n-\t\t\t\tbreak;\n-\t\t\t} else {\n+\t\tchosen = list_and_choose(&menu_opts, &menu_stuff);\n+\n+\t\tif (*chosen != EOF) {\n+\t\t\tint ret;\n+\t\t\tret = menus[*chosen].fn();\n+\t\t\tif (ret != MENU_RETURN_NO_LOOP) {\n+\t\t\t\tfree(chosen);\n+\t\t\t\tchosen = NULL;\n+\t\t\t\tif (!del_list.nr) {\n+\t\t\t\t\tclean_print_color(CLEAN_COLOR_ERROR);\n+\t\t\t\t\tprintf_ln(_(\"No more files to clean, exiting.\"));\n+\t\t\t\t\tclean_print_color(CLEAN_COLOR_RESET);\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t} else {\n+\t\t\tquit_cmd();\n \t\t}\n-\t}\n \n-\tstrbuf_release(&confirm);\n+\t\tfree(chosen);\n+\t\tchosen = NULL;\n+\t\tbreak;\n+\t}\n }\n \n int cmd_clean(int argc, const char **argv, const char *prefix)\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221814","messageId":"36d2d6db4e65145bab6c85d7952f2cc068de65c5.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 11/16] git-clean: add filter by pattern interactive action","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:35Z","receivedAt":"2013-06-24T15:21:35Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Add a new action for interactive git-clean: filter by pattern. When the\nuser chooses this action, user can input space-separated patterns (the\nsame syntax as gitignore), and each clean candidate that matches with\none of the patterns will be excluded from cleaning. When the user feels\nit's OK, presses ENTER and backs to the confirmation dialog.\n\nSigned-off-by: Jiang Xin \nSuggested-by: Junio C Hamano \nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 68 insertions(+)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex df887..36369 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -614,6 +614,72 @@ static int clean_cmd(void)\n \treturn MENU_RETURN_NO_LOOP;\n }\n \n+static int filter_by_patterns_cmd(void)\n+{\n+\tstruct dir_struct dir;\n+\tstruct strbuf confirm = STRBUF_INIT;\n+\tstruct strbuf **ignore_list;\n+\tstruct string_list_item *item;\n+\tstruct exclude_list *el;\n+\tint changed = -1, i;\n+\n+\tfor (;;) {\n+\t\tif (!del_list.nr)\n+\t\t\tbreak;\n+\n+\t\tif (changed)\n+\t\t\tpretty_print_dels();\n+\n+\t\tclean_print_color(CLEAN_COLOR_PROMPT);\n+\t\tprintf(_(\"Input ignore patterns>> \"));\n+\t\tclean_print_color(CLEAN_COLOR_RESET);\n+\t\tif (strbuf_getline(&confirm, stdin, '\\n') != EOF)\n+\t\t\tstrbuf_trim(&confirm);\n+\t\telse\n+\t\t\tputchar('\\n');\n+\n+\t\t/* quit filter_by_pattern mode if press ENTER or Ctrl-D */\n+\t\tif (!confirm.len)\n+\t\t\tbreak;\n+\n+\t\tmemset(&dir, 0, sizeof(dir));\n+\t\tel = add_exclude_list(&dir, EXC_CMDL, \"manual exclude\");\n+\t\tignore_list = strbuf_split_max(&confirm, ' ', 0);\n+\n+\t\tfor (i = 0; ignore_list[i]; i++) {\n+\t\t\tstrbuf_trim(ignore_list[i]);\n+\t\t\tif (!ignore_list[i]->len)\n+\t\t\t\tcontinue;\n+\n+\t\t\tadd_exclude(ignore_list[i]->buf, \"\", 0, el, -(i+1));\n+\t\t}\n+\n+\t\tchanged = 0;\n+\t\tfor_each_string_list_item(item, &del_list) {\n+\t\t\tint dtype = DT_UNKNOWN;\n+\n+\t\t\tif (is_excluded(&dir, item->string, &dtype)) {\n+\t\t\t\t*item->string = '\\0';\n+\t\t\t\tchanged++;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (changed) {\n+\t\t\tstring_list_remove_empty_items(&del_list, 0);\n+\t\t} else {\n+\t\t\tclean_print_color(CLEAN_COLOR_ERROR);\n+\t\t\tprintf_ln(_(\"WARNING: Cannot find items matched by: %s\"), confirm.buf);\n+\t\t\tclean_print_color(CLEAN_COLOR_RESET);\n+\t\t}\n+\n+\t\tstrbuf_list_free(ignore_list);\n+\t\tclear_directory(&dir);\n+\t}\n+\n+\tstrbuf_release(&confirm);\n+\treturn 0;\n+}\n+\n static int quit_cmd(void)\n {\n \tstring_list_clear(&del_list, 0);\n@@ -626,6 +692,7 @@ static int help_cmd(void)\n \tclean_print_color(CLEAN_COLOR_HELP);\n \tprintf_ln(_(\n \t\t \"clean - start cleaning\\n\"\n+\t\t \"filter by pattern - exclude items from deletion\\n\"\n \t\t \"quit - stop cleaning\\n\"\n \t\t \"help - this screen\\n\"\n \t\t \"? - help for prompt selection\"\n@@ -641,6 +708,7 @@ static void interactive_main_loop(void)\n \t\tstruct menu_stuff menu_stuff;\n \t\tstruct menu_item menus[] = {\n \t\t\t{'c', \"clean\",\t\t\t0, clean_cmd},\n+\t\t\t{'f', \"filter by pattern\",\t0, filter_by_patterns_cmd},\n \t\t\t{'q', \"quit\",\t\t\t0, quit_cmd},\n \t\t\t{'h', \"help\",\t\t\t0, help_cmd},\n \t\t};\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221813","messageId":"b0da2c8e4aa3767335c4d3a95402f43644c2a691.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 12/16] git-clean: add select by numbers interactive action","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:36Z","receivedAt":"2013-06-24T15:21:36Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Draw a multiple choice menu using `list_and_choose` to select items\nto be deleted by numbers.\n\nUser can input:\n\n * 1,5-7 : select 1,5,6,7 items to be deleted\n * * : select all items to be deleted\n * -* : unselect all, nothing will be deleted\n * : (empty) finish selecting, and return back to main menu\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 39 +++++++++++++++++++++++++++++++++++++++\n 1 file changed, 39 insertions(+)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 36369..643a5e 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -680,6 +680,43 @@ static int filter_by_patterns_cmd(void)\n \treturn 0;\n }\n \n+static int select_by_numbers_cmd(void)\n+{\n+\tstruct menu_opts menu_opts;\n+\tstruct menu_stuff menu_stuff;\n+\tstruct string_list_item *items;\n+\tint *chosen;\n+\tint i, j;\n+\n+\tmenu_opts.header = NULL;\n+\tmenu_opts.prompt = N_(\"Select items to delete\");\n+\tmenu_opts.flags = 0;\n+\n+\tmenu_stuff.type = MENU_STUFF_TYPE_STRING_LIST;\n+\tmenu_stuff.stuff = &del_list;\n+\tmenu_stuff.nr = del_list.nr;\n+\n+\tchosen = list_and_choose(&menu_opts, &menu_stuff);\n+\titems = del_list.items;\n+\tfor (i = 0, j = 0; i < del_list.nr; i++) {\n+\t\tif (i < chosen[j]) {\n+\t\t\t*(items[i].string) = '\\0';\n+\t\t} else if (i == chosen[j]) {\n+\t\t\t/* delete selected item */\n+\t\t\tj++;\n+\t\t\tcontinue;\n+\t\t} else {\n+\t\t\t/* end of chosen (chosen[j] == EOF), won't delete */\n+\t\t\t*(items[i].string) = '\\0';\n+\t\t}\n+\t}\n+\n+\tstring_list_remove_empty_items(&del_list, 0);\n+\n+\tfree(chosen);\n+\treturn 0;\n+}\n+\n static int quit_cmd(void)\n {\n \tstring_list_clear(&del_list, 0);\n@@ -693,6 +730,7 @@ static int help_cmd(void)\n \tprintf_ln(_(\n \t\t \"clean - start cleaning\\n\"\n \t\t \"filter by pattern - exclude items from deletion\\n\"\n+\t\t \"select by numbers - select items to be deleted by numbers\\n\"\n \t\t \"quit - stop cleaning\\n\"\n \t\t \"help - this screen\\n\"\n \t\t \"? - help for prompt selection\"\n@@ -709,6 +747,7 @@ static void interactive_main_loop(void)\n \t\tstruct menu_item menus[] = {\n \t\t\t{'c', \"clean\",\t\t\t0, clean_cmd},\n \t\t\t{'f', \"filter by pattern\",\t0, filter_by_patterns_cmd},\n+\t\t\t{'s', \"select by numbers\",\t0, select_by_numbers_cmd},\n \t\t\t{'q', \"quit\",\t\t\t0, quit_cmd},\n \t\t\t{'h', \"help\",\t\t\t0, help_cmd},\n \t\t};\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221816","messageId":"061b72a5ad3ac9c605df503034952e85aeb3ce98.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 13/16] git-clean: add ask each interactive action","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:37Z","receivedAt":"2013-06-24T15:21:37Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Add a new action for interactive git-clean: ask each. It's just like\nthe \"rm -i\" command, that the user must confirm one by one for each\nfile or directory to be cleaned.\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Junio C Hamano \n---\n builtin/clean.c | 36 ++++++++++++++++++++++++++++++++++++\n 1 file changed, 36 insertions(+)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 643a5e..bf03a 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -717,6 +717,40 @@ static int select_by_numbers_cmd(void)\n \treturn 0;\n }\n \n+static int ask_each_cmd(void)\n+{\n+\tstruct strbuf confirm = STRBUF_INIT;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct string_list_item *item;\n+\tconst char *qname;\n+\tint changed = 0, eof = 0;\n+\n+\tfor_each_string_list_item(item, &del_list) {\n+\t\t/* Ctrl-D should stop removing files */\n+\t\tif (!eof) {\n+\t\t\tqname = quote_path_relative(item->string, NULL, &buf);\n+\t\t\tprintf(_(\"remove %s? \"), qname);\n+\t\t\tif (strbuf_getline(&confirm, stdin, '\\n') != EOF) {\n+\t\t\t\tstrbuf_trim(&confirm);\n+\t\t\t} else {\n+\t\t\t\tputchar('\\n');\n+\t\t\t\teof = 1;\n+\t\t\t}\n+\t\t}\n+\t\tif (!confirm.len || strncasecmp(confirm.buf, \"yes\", confirm.len)) {\n+\t\t\t*item->string = '\\0';\n+\t\t\tchanged++;\n+\t\t}\n+\t}\n+\n+\tif (changed)\n+\t\tstring_list_remove_empty_items(&del_list, 0);\n+\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&confirm);\n+\treturn MENU_RETURN_NO_LOOP;\n+}\n+\n static int quit_cmd(void)\n {\n \tstring_list_clear(&del_list, 0);\n@@ -731,6 +765,7 @@ static int help_cmd(void)\n \t\t \"clean - start cleaning\\n\"\n \t\t \"filter by pattern - exclude items from deletion\\n\"\n \t\t \"select by numbers - select items to be deleted by numbers\\n\"\n+\t\t \"ask each - confirm each deletion (like \\\"rm -i\\\")\\n\"\n \t\t \"quit - stop cleaning\\n\"\n \t\t \"help - this screen\\n\"\n \t\t \"? - help for prompt selection\"\n@@ -748,6 +783,7 @@ static void interactive_main_loop(void)\n \t\t\t{'c', \"clean\",\t\t\t0, clean_cmd},\n \t\t\t{'f', \"filter by pattern\",\t0, filter_by_patterns_cmd},\n \t\t\t{'s', \"select by numbers\",\t0, select_by_numbers_cmd},\n+\t\t\t{'a', \"ask each\",\t\t0, ask_each_cmd},\n \t\t\t{'q', \"quit\",\t\t\t0, quit_cmd},\n \t\t\t{'h', \"help\",\t\t\t0, help_cmd},\n \t\t};\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221815","messageId":"28725b96320fbc26322860c7213fa20ae7b4863c.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 14/16] git-clean: add documentation for interactive git-clean","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:38Z","receivedAt":"2013-06-24T15:21:38Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Add new section \"Interactive mode\" for documentation of interactive\ngit-clean.\n\nSigned-off-by: Jiang Xin \nHelped-by: Eric Sunshine \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-clean.txt | 65 +++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 63 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt\nindex 186e34..5bf76 100644\n--- a/Documentation/git-clean.txt\n+++ b/Documentation/git-clean.txt\n@@ -39,8 +39,8 @@ OPTIONS\n \n -i::\n --interactive::\n-\tShow what would be done and the user must confirm before actually\n-\tcleaning.\n+\tShow what would be done and clean files interactively. See\n+\t``Interactive mode'' for details.\n \n -n::\n --dry-run::\n@@ -69,6 +69,67 @@ OPTIONS\n \tRemove only files ignored by Git. This may be useful to rebuild\n \teverything from scratch, but keep manually created files.\n \n+Interactive mode\n+----------------\n+When the command enters the interactive mode, it shows the\n+files and directories to be cleaned, and goes into its\n+interactive command loop.\n+\n+The command loop shows the list of subcommands available, and\n+gives a prompt \"What now> \". In general, when the prompt ends\n+with a single '>', you can pick only one of the choices given\n+and type return, like this:\n+\n+------------\n+ *** Commands ***\n+ 1: clean 2: filter by pattern 3: select by numbers\n+ 4: ask each 5: quit 6: help\n+ What now> 1\n+------------\n+\n+You also could say `c` or `clean` above as long as the choice is unique.\n+\n+The main command loop has 6 subcommands.\n+\n+clean::\n+\n+ Start cleaning files and directories, and then quit.\n+\n+filter by pattern::\n+\n+ This shows the files and directories to be deleted and issues an\n+ \"Input ignore patterns>>\" prompt. You can input space-seperated\n+ patterns to exclude files and directories from deletion.\n+ E.g. \"*.c *.h\" will excludes files end with \".c\" and \".h\" from\n+ deletion. When you are satisfied with the filtered result, press\n+ ENTER (empty) back to the main menu.\n+\n+select by numbers::\n+\n+ This shows the files and directories to be deleted and issues an\n+ \"Select items to delete>>\" prompt. When the prompt ends with double\n+ '>>' like this, you can make more than one selection, concatenated\n+ with whitespace or comma. Also you can say ranges. E.g. \"2-5 7,9\"\n+ to choose 2,3,4,5,7,9 from the list. If the second number in a\n+ range is omitted, all remaining patches are taken. E.g. \"7-\" to\n+ choose 7,8,9 from the list. You can say '*' to choose everything.\n+ Also when you are satisfied with the filtered result, press ENTER\n+ (empty) back to the main menu.\n+\n+ask each::\n+\n+ This will start to clean, and you must confirm one by one in order\n+ to delete items. Please note that this action is not as efficient\n+ as the above two actions.\n+\n+quit::\n+\n+ This lets you quit without do cleaning.\n+\n+help::\n+\n+ Show brief usage of interactive git-clean.\n+\n SEE ALSO\n --------\n linkgit:gitignore[5]\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221817","messageId":"b1f44d88d17a567d3d1afcb2db85fbfa037745b3.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 15/16] test: add t7301 for git-clean--interactive","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:39Z","receivedAt":"2013-06-24T15:21:39Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Add test cases for git-clean--interactive.\n\nSigned-off-by: Jiang Xin \nSigned-off-by: Junio C Hamano \n---\n t/t7301-clean-interactive.sh | 439 +++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 439 insertions(+)\n create mode 100755 t/t7301-clean-interactive.sh\n\ndiff --git a/t/t7301-clean-interactive.sh b/t/t7301-clean-interactive.sh\nnew file mode 100755\nindex 00000..4e605\n--- /dev/null\n+++ b/t/t7301-clean-interactive.sh\n@@ -0,0 +1,439 @@\n+#!/bin/sh\n+\n+test_description='git clean -i basic tests'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\n+\tmkdir -p src &&\n+\ttouch src/part1.c Makefile &&\n+\techo build >.gitignore &&\n+\techo \\*.o >>.gitignore &&\n+\tgit add . &&\n+\tgit commit -m setup &&\n+\ttouch src/part2.c README &&\n+\tgit add .\n+\n+'\n+\n+test_expect_success 'git clean -i (clean)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\techo c | git clean -i &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest ! -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest ! -f src/part3.c &&\n+\ttest ! -f src/part3.h &&\n+\ttest ! -f src/part4.c &&\n+\ttest ! -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -i (quit)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\techo q | git clean -i &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -i (Ctrl+D)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\techo \"\\04\" | git clean -i &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (filter all)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo f; echo \"*\"; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (filter patterns)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo f; echo \"part3.* *.out\"; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest -f a.out &&\n+\ttest ! -f docs/manual.txt &&\n+\ttest -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest ! -f src/part4.c &&\n+\ttest ! -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (filter patterns 2)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo f; echo \"* !*.out\"; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest ! -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (select - all)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo s; echo \"*\"; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest ! -f a.out &&\n+\ttest ! -f docs/manual.txt &&\n+\ttest ! -f src/part3.c &&\n+\ttest ! -f src/part3.h &&\n+\ttest ! -f src/part4.c &&\n+\ttest ! -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (select - none)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo s; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (select - number)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo s; echo 3; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest ! -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (select - number 2)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo s; echo 2 3; echo 5; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest -f a.out &&\n+\ttest ! -f docs/manual.txt &&\n+\ttest ! -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest ! -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (select - number 3)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo s; echo 3,4 5; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest ! -f src/part3.c &&\n+\ttest ! -f src/part3.h &&\n+\ttest ! -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (select - range)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo s; echo 1,3-4; echo 2; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest ! -f a.out &&\n+\ttest ! -f src/part3.c &&\n+\ttest ! -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest ! -f docs/manual.txt &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (select - range 2)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo s; echo 4- 1; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest ! -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest -f src/part3.c &&\n+\ttest ! -f src/part3.h &&\n+\ttest ! -f src/part4.c &&\n+\ttest ! -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (inverse select)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo s; echo \"*\"; echo -5- 1 -2; echo; echo c) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest ! -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest ! -f src/part3.c &&\n+\ttest ! -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (ask)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo a; echo Y; echo y; echo no; echo yes; echo bad; echo) | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest ! -f a.out &&\n+\ttest ! -f docs/manual.txt &&\n+\ttest -f src/part3.c &&\n+\ttest ! -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id (ask - Ctrl+D)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(echo a; echo Y; echo no; echo yes; echo \"\\04\") | \\\n+\tgit clean -id &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest ! -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest ! -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id with prefix and path (filter)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(cd build/ && \\\n+\t (echo f; echo \"docs\"; echo \"*.h\"; echo ; echo c) | \\\n+\t git clean -id ..) &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest ! -f a.out &&\n+\ttest -f docs/manual.txt &&\n+\ttest ! -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest ! -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id with prefix and path (select by name)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(cd build/ && \\\n+\t (echo s; echo \"../docs/\"; echo \"../src/part3.c\"; \\\n+\t echo \"../src/part4.c\"; echo; echo c) | \\\n+\t git clean -id ..) &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest -f a.out &&\n+\ttest ! -f docs/manual.txt &&\n+\ttest ! -f src/part3.c &&\n+\ttest -f src/part3.h &&\n+\ttest ! -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_expect_success 'git clean -id with prefix and path (ask)' '\n+\n+\tmkdir -p build docs &&\n+\ttouch a.out src/part3.c src/part3.h src/part4.c src/part4.h \\\n+\tdocs/manual.txt obj.o build/lib.so &&\n+\t(cd build/ && \\\n+\t (echo a; echo Y; echo y; echo no; echo yes; echo bad; echo) | \\\n+\t git clean -id ..) &&\n+\ttest -f Makefile &&\n+\ttest -f README &&\n+\ttest -f src/part1.c &&\n+\ttest -f src/part2.c &&\n+\ttest ! -f a.out &&\n+\ttest ! -f docs/manual.txt &&\n+\ttest -f src/part3.c &&\n+\ttest ! -f src/part3.h &&\n+\ttest -f src/part4.c &&\n+\ttest -f src/part4.h &&\n+\ttest -f obj.o &&\n+\ttest -f build/lib.so\n+\n+'\n+\n+test_done\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221818","messageId":"7b6237d76d53c240daf4641a00a09af5135fbba7.1372087065.git.worldhello.net@gmail.com","threadId":"34120","inReplyTo":"cover.1372087065.git.worldhello.net@gmail.com","subject":"[PATCH v14 16/16] test: run testcases with POSIX absolute paths on Windows","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-24T15:21:40Z","receivedAt":"2013-06-24T15:21:40Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"Add new subcommand \"mingw_path\" in test-path-utils, so that we can get\nthe expected absolute paths on Windows. For example:\n\n COMMAND LINE Linux Windows\n ================================== ===== ===============\n test-path-utils mingw_path / / C:/msysgit\n test-path-utils mingw_path /a/b/ /a/b/ C:/msysgit/a/b/\n\nWith this utility, most skipped test cases in t0060 can be runcorrectly\non Windows.\n\nSigned-off-by: Jiang Xin \n---\n t/t0060-path-utils.sh | 73 ++++++++++++++++++++++++++++++++++-----------------\n test-path-utils.c | 5 ++++\n 2 files changed, 54 insertions(+), 24 deletions(-)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 4deec..dac84 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -7,14 +7,39 @@ test_description='Test various path utilities'\n \n . ./test-lib.sh\n \n+mingw_path() {\n+\tcase $2 in\n+\tNO_MINGW)\n+\t\techo \"$1\"\n+\t\t;;\n+\t*)\n+\t\ttest-path-utils mingw_path \"$1\"\n+\t\t;;\n+\tesac\n+}\n+\n+get_prereq_flag() {\n+\tcase $1 in\n+\tPOSIX)\n+\t\techo $1\n+\t\t;;\n+\t*)\n+\t\t;;\n+\tesac\n+}\n+\n norm_path() {\n-\ttest_expect_success $3 \"normalize path: $1 => $2\" \\\n-\t\"test \\\"\\$(test-path-utils normalize_path_copy '$1')\\\" = '$2'\"\n+\texpected=$(mingw_path \"$2\" \"$3\")\n+\tprereq=$(get_prereq_flag $3)\n+\ttest_expect_success $prereq \"normalize path: $1 => $2\" \\\n+\t\"test \\\"\\$(test-path-utils normalize_path_copy '$1')\\\" = '$expected'\"\n }\n \n relative_path() {\n-\ttest_expect_success $4 \"relative path: $1 $2 => $3\" \\\n-\t\"test \\\"\\$(test-path-utils relative_path '$1' '$2')\\\" = '$3'\"\n+\texpected=$(mingw_path \"$3\" \"$4\")\n+\tprereq=$(get_prereq_flag $4)\n+\ttest_expect_success $prereq \"relative path: $1 $2 => $3\" \\\n+\t\"test \\\"\\$(test-path-utils relative_path '$1' '$2')\\\" = '$expected'\"\n }\n \n # On Windows, we are using MSYS's bash, which mangles the paths.\n@@ -39,8 +64,8 @@ ancestor() {\n \t test \\\"\\$actual\\\" = '$expected'\"\n }\n \n-# Absolute path tests must be skipped on Windows because due to path mangling\n-# the test program never sees a POSIX-style absolute path\n+# Some absolute path tests should be skipped on Windows due to path mangling\n+# on POSIX-style absolute paths\n case $(uname -s) in\n *MINGW*)\n \t;;\n@@ -73,10 +98,10 @@ norm_path d1/s1//../s2/../../d2 d2\n norm_path d1/.../d2 d1/.../d2\n norm_path d1/..././../d2 d1/d2\n \n-norm_path / / POSIX\n-norm_path // / POSIX\n-norm_path /// / POSIX\n-norm_path /. / POSIX\n+norm_path / /\n+norm_path // / NO_MINGW\n+norm_path /// / NO_MINGW\n+norm_path /. /\n norm_path /./ / POSIX\n norm_path /./.. ++failed++ POSIX\n norm_path /../. ++failed++ POSIX\n@@ -84,19 +109,19 @@ norm_path /./../.// ++failed++ POSIX\n norm_path /dir/.. / POSIX\n norm_path /dir/sub/../.. / POSIX\n norm_path /dir/sub/../../.. ++failed++ POSIX\n-norm_path /dir /dir POSIX\n-norm_path /dir// /dir/ POSIX\n-norm_path /./dir /dir POSIX\n-norm_path /dir/. /dir/ POSIX\n-norm_path /dir///./ /dir/ POSIX\n-norm_path /dir//sub/.. /dir/ POSIX\n-norm_path /dir/sub/../ /dir/ POSIX\n+norm_path /dir /dir\n+norm_path /dir// /dir/\n+norm_path /./dir /dir\n+norm_path /dir/. /dir/\n+norm_path /dir///./ /dir/\n+norm_path /dir//sub/.. /dir/\n+norm_path /dir/sub/../ /dir/\n norm_path //dir/sub/../. /dir/ POSIX\n-norm_path /dir/s1/../s2/ /dir/s2/ POSIX\n-norm_path /d1/s1///s2/..//../s3/ /d1/s3/ POSIX\n-norm_path /d1/s1//../s2/../../d2 /d2 POSIX\n-norm_path /d1/.../d2 /d1/.../d2 POSIX\n-norm_path /d1/..././../d2 /d1/d2 POSIX\n+norm_path /dir/s1/../s2/ /dir/s2/\n+norm_path /d1/s1///s2/..//../s3/ /d1/s3/\n+norm_path /d1/s1//../s2/../../d2 /d2\n+norm_path /d1/.../d2 /d1/.../d2\n+norm_path /d1/..././../d2 /d1/d2\n \n ancestor / / -1\n ancestor /foo / 0\n@@ -197,8 +222,8 @@ relative_path /a\t/a/b\t\t../\n relative_path /\t\t/a/b/\t\t../../\n relative_path /a/c\t/a/b/\t\t../c\n relative_path /a/c\t/a/b\t\t../c\n-relative_path /a/b\t\"\"\t/a/b\tPOSIX\n-relative_path /a/b \t\"\"\t/a/b\tPOSIX\n+relative_path /a/b\t\"\"\t/a/b\n+relative_path /a/b \t\"\"\t/a/b\n relative_path \"\"\t/a/b\t\t./\n relative_path \"\"\t\"\"\t./\n relative_path \"\"\t\"\"\t./\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex 95ef4..699ef 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -116,6 +116,11 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n+\tif (argc == 3 && !strcmp(argv[1], \"mingw_path\")) {\n+\t\tputs(argv[2]);\n+\t\treturn 0;\n+\t}\n+\n \tif (argc == 4 && !strcmp(argv[1], \"relative_path\")) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \t\tconst char *abs, *base, *rel;\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221878","messageId":"51C89655.4070208@kdbg.org","threadId":"34120","inReplyTo":"7b6237d76d53c240daf4641a00a09af5135fbba7.1372087065.git.worldhello.net@gmail.com","subject":"Re: [PATCH v14 16/16] test: run testcases with POSIX absolute paths on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-06-24T18:56:21Z","receivedAt":"2013-06-24T18:56:21Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 24.06.2013 17:21, schrieb Jiang Xin:\n> Add new subcommand \"mingw_path\" in test-path-utils, so that we can get\n> the expected absolute paths on Windows. For example:\n> \n> COMMAND LINE Linux Windows\n> ================================== ===== ===============\n> test-path-utils mingw_path / / C:/msysgit\n> test-path-utils mingw_path /a/b/ /a/b/ C:/msysgit/a/b/\n> \n> With this utility, most skipped test cases in t0060 can be runcorrectly\n> on Windows.\n\nThanks for working on this. Did you have an opportunity to test on Windows?\n\n> +mingw_path() {\n> +\tcase $2 in\n> +\tNO_MINGW)\n> +\t\techo \"$1\"\n> +\t\t;;\n> +\t*)\n> +\t\ttest-path-utils mingw_path \"$1\"\n> +\t\t;;\n> +\tesac\n> +}\n> +\n> +get_prereq_flag() {\n> +\tcase $1 in\n> +\tPOSIX)\n> +\t\techo $1\n> +\t\t;;\n> +\t*)\n> +\t\t;;\n> +\tesac\n> +}\n\nIt took me a while to notice that the token POSIX serves a different\npurpose than NO_MINGW: POSIX is only needed to skip a test, NO_MINGW is\nonly used to avoid a test-path-utils call. The reason for the complexity\nis that you put both tokens in the same parameter position in the\ninvocations of the test cases.\n\nYou use NO_MINGW only twice. I would just skip these two tests, then the\nparameter position is reserved for the prerequisite, and things stay simple.\n\n> +\n> norm_path() {\n> -\ttest_expect_success $3 \"normalize path: $1 => $2\" \\\n> -\t\"test \\\"\\$(test-path-utils normalize_path_copy '$1')\\\" = '$2'\"\n> +\texpected=$(mingw_path \"$2\" \"$3\")\n> +\tprereq=$(get_prereq_flag $3)\n> +\ttest_expect_success $prereq \"normalize path: $1 => $2\" \\\n> +\t\"test \\\"\\$(test-path-utils normalize_path_copy '$1')\\\" = '$expected'\"\n> }\n> \n> relative_path() {\n> -\ttest_expect_success $4 \"relative path: $1 $2 => $3\" \\\n> -\t\"test \\\"\\$(test-path-utils relative_path '$1' '$2')\\\" = '$3'\"\n> +\texpected=$(mingw_path \"$3\" \"$4\")\n> +\tprereq=$(get_prereq_flag $4)\n> +\ttest_expect_success $prereq \"relative path: $1 $2 => $3\" \\\n> +\t\"test \\\"\\$(test-path-utils relative_path '$1' '$2')\\\" = '$expected'\"\n> }\n> \n> # On Windows, we are using MSYS's bash, which mangles the paths.\n> @@ -39,8 +64,8 @@ ancestor() {\n> \t test \\\"\\$actual\\\" = '$expected'\"\n> }\n> \n> -# Absolute path tests must be skipped on Windows because due to path mangling\n> -# the test program never sees a POSIX-style absolute path\n> +# Some absolute path tests should be skipped on Windows due to path mangling\n> +# on POSIX-style absolute paths\n> case $(uname -s) in\n> *MINGW*)\n> \t;;\n> @@ -73,10 +98,10 @@ norm_path d1/s1//../s2/../../d2 d2\n> norm_path d1/.../d2 d1/.../d2\n> norm_path d1/..././../d2 d1/d2\n> \n> -norm_path / / POSIX\n> -norm_path // / POSIX\n> -norm_path /// / POSIX\n> -norm_path /. / POSIX\n> +norm_path / /\n> +norm_path // / NO_MINGW\n> +norm_path /// / NO_MINGW\n> +norm_path /. /\n> norm_path /./ / POSIX\n> norm_path /./.. ++failed++ POSIX\n> norm_path /../. ++failed++ POSIX\n> @@ -84,19 +109,19 @@ norm_path /./../.// ++failed++ POSIX\n> norm_path /dir/.. / POSIX\n> norm_path /dir/sub/../.. / POSIX\n> norm_path /dir/sub/../../.. ++failed++ POSIX\n> -norm_path /dir /dir POSIX\n> -norm_path /dir// /dir/ POSIX\n> -norm_path /./dir /dir POSIX\n> -norm_path /dir/. /dir/ POSIX\n> -norm_path /dir///./ /dir/ POSIX\n> -norm_path /dir//sub/.. /dir/ POSIX\n> -norm_path /dir/sub/../ /dir/ POSIX\n> +norm_path /dir /dir\n> +norm_path /dir// /dir/\n> +norm_path /./dir /dir\n> +norm_path /dir/. /dir/\n> +norm_path /dir///./ /dir/\n> +norm_path /dir//sub/.. /dir/\n> +norm_path /dir/sub/../ /dir/\n> norm_path //dir/sub/../. /dir/ POSIX\n> -norm_path /dir/s1/../s2/ /dir/s2/ POSIX\n> -norm_path /d1/s1///s2/..//../s3/ /d1/s3/ POSIX\n> -norm_path /d1/s1//../s2/../../d2 /d2 POSIX\n> -norm_path /d1/.../d2 /d1/.../d2 POSIX\n> -norm_path /d1/..././../d2 /d1/d2 POSIX\n> +norm_path /dir/s1/../s2/ /dir/s2/\n> +norm_path /d1/s1///s2/..//../s3/ /d1/s3/\n> +norm_path /d1/s1//../s2/../../d2 /d2\n> +norm_path /d1/.../d2 /d1/.../d2\n> +norm_path /d1/..././../d2 /d1/d2\n> \n> ancestor / / -1\n> ancestor /foo / 0\n> @@ -197,8 +222,8 @@ relative_path /a\t/a/b\t\t../\n> relative_path /\t\t/a/b/\t\t../../\n> relative_path /a/c\t/a/b/\t\t../c\n> relative_path /a/c\t/a/b\t\t../c\n> -relative_path /a/b\t\"\"\t/a/b\tPOSIX\n> -relative_path /a/b \t\"\"\t/a/b\tPOSIX\n> +relative_path /a/b\t\"\"\t/a/b\n> +relative_path /a/b \t\"\"\t/a/b\n> relative_path \"\"\t/a/b\t\t./\n> relative_path \"\"\t\"\"\t./\n> relative_path \"\"\t\"\"\t./\n> diff --git a/test-path-utils.c b/test-path-utils.c\n> index 95ef4..699ef 100644\n> --- a/test-path-utils.c\n> +++ b/test-path-utils.c\n> @@ -116,6 +116,11 @@ int main(int argc, char **argv)\n> \t\treturn 0;\n> \t}\n> \n> +\tif (argc == 3 && !strcmp(argv[1], \"mingw_path\")) {\n> +\t\tputs(argv[2]);\n> +\t\treturn 0;\n> +\t}\n> +\n> \tif (argc == 4 && !strcmp(argv[1], \"relative_path\")) {\n> \t\tstruct strbuf sb = STRBUF_INIT;\n> \t\tconst char *abs, *base, *rel;\n> \n"},{"id":"221879","messageId":"7vli5z8ha0.fsf@alter.siamese.dyndns.org","threadId":"34120","inReplyTo":"85c9aa4d05dba4320a7cc85eae7570e0f450e58e.1372087065.git.worldhello.net@gmail.com","subject":"Re: [PATCH v14 01/16] test: add test cases for relative_path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-24T19:01:27Z","receivedAt":"2013-06-24T19:01:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin writes:\n\n> diff --git a/test-path-utils.c b/test-path-utils.c\n> index 0092cb..dcc530 100644\n> --- a/test-path-utils.c\n> +++ b/test-path-utils.c\n> @@ -28,6 +28,19 @@ static int normalize_ceiling_entry(struct string_list_item *item, void *unused)\n> \treturn 1;\n> }\n> \n> +static void normalize_argv_string(const char **var, const char *input)\n> +{\n> +\tif (!strcmp(input, \"\"))\n> +\t\t*var = NULL;\n> +\telse if (!strcmp(input, \"\"))\n> +\t\t*var = \"\";\n> +\telse\n> +\t\t*var = input;\n> +\n> +\tif (*var && (**var == '<' || **var == '('))\n> +\t\tdie(\"Bad value: %s\\n\", input);\n> +}\n> +\n\nIf you have to munge the input string like this anyway, perhaps you\ncan work around the command line mangling done by Windows bash\nruntime, perhaps add something like:\n\n\tif (*input == '_')\n \tinput++;\n\nand then protecting the path with the underscore, like so?\n\n\trelative_path _/a/b/c/\t_/a/b/\t\tc/\n\nWouldn't that let you avoid having to handle \"POSIX\" prereq for\nthese paths?\n"},{"id":"221880","messageId":"7vhagn8gtz.fsf@alter.siamese.dyndns.org","threadId":"34120","inReplyTo":"654b2387c27470c3dc81169e8f43c231aa9504ce.1372087065.git.worldhello.net@gmail.com","subject":"Re: [PATCH v14 03/16] quote.c: remove path_relative, use relative_path instead","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-24T19:11:04Z","receivedAt":"2013-06-24T19:11:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin writes:\n\n> Since there is an enhanced version of relative_path() in path.c,\n> remove duplicate counterpart path_relative() in quote.c.\n\nThere is no nice comparison chart before and after like you had in\npatch 02/16?\n\n> void write_name_quoted_relative(const char *name, size_t len,\n> \t\t\t\tconst char *prefix, size_t prefix_len,\n> \t\t\t\tFILE *fp, int terminator)\n> {\n> \tstruct strbuf sb = STRBUF_INIT;\n> \n> -\tname = path_relative(name, len, &sb, prefix, prefix_len);\n> +\tname = relative_path(name, prefix, &sb);\n\nAre we sure nobody calls prefix_len pointing into the middle of\nstring, not at the end of \"prefix\"? This is unsafe for such a\ncaller, and to make sure we catch them, we should remove the\nnow-unused prefix_len parameter from this function.\n\n> \twrite_name_quoted(name, fp, terminator);\n> \n> \tstrbuf_release(&sb);\n> }\n> \n> -/*\n> - * Give path as relative to prefix.\n> - *\n> - * The strbuf may or may not be used, so do not assume it contains the\n> - * returned path.\n> - */\n> -static const char *path_relative(const char *in, int len,\n> -\t\t\t\t struct strbuf *sb, const char *prefix,\n> -\t\t\t\t int prefix_len)\n> -{\n> -\tint off, i;\n> -\n> -\tif (len < 0)\n> -\t\tlen = strlen(in);\n> -\tif (prefix_len < 0) {\n> -\t\tif (prefix)\n> -\t\t\tprefix_len = strlen(prefix);\n> -\t\telse\n> -\t\t\tprefix_len = 0;\n> -\t}\n> -\n> -\toff = 0;\n> -\ti = 0;\n> -\twhile (i < prefix_len && i < len && prefix[i] == in[i]) {\n> -\t\tif (prefix[i] == '/')\n> -\t\t\toff = i + 1;\n> -\t\ti++;\n> -\t}\n> -\tin += off;\n> -\tlen -= off;\n> -\n> -\tif (i >= prefix_len)\n> -\t\treturn in;\n> -\n> -\tstrbuf_reset(sb);\n> -\tstrbuf_grow(sb, len);\n> -\n> -\twhile (i < prefix_len) {\n> -\t\tif (prefix[i] == '/')\n> -\t\t\tstrbuf_addstr(sb, \"../\");\n> -\t\ti++;\n> -\t}\n> -\tstrbuf_add(sb, in, len);\n> -\n> -\treturn sb->buf;\n> -}\n> -\n> /* quote path as relative to the given prefix */\n> char *quote_path_relative(const char *in, int len,\n> \t\t\t struct strbuf *out, const char *prefix)\n> {\n> \tstruct strbuf sb = STRBUF_INIT;\n> -\tconst char *rel = path_relative(in, len, &sb, prefix, -1);\n> +\tconst char *rel = relative_path(in, prefix, &sb);\n> \tstrbuf_reset(out);\n> \tquote_c_style_counted(rel, strlen(rel), out, NULL, 0);\n> \tstrbuf_release(&sb);\n"},{"id":"221881","messageId":"7vd2rb8gmc.fsf@alter.siamese.dyndns.org","threadId":"34120","inReplyTo":"c7b9df81fad6a5b87400ae7beb6e6c2b7d48a67b.1372087065.git.worldhello.net@gmail.com","subject":"Re: [PATCH v14 04/16] Refactor quote_path_relative, remove unused params","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-24T19:15:39Z","receivedAt":"2013-06-24T19:15:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin writes:\n\n> After substitute path_relative() in quote.c with relative_path() from\n> path.c, parameters (such as len and prefix_len) are obsolete in function\n> quote_path_relative(). Remove unused parameters and change the order of\n> parameters for quote_path_relative() function.\n> ...\n> diff --git a/builtin/clean.c b/builtin/clean.c\n> index 04e39..f77f95 100644\n> --- a/builtin/clean.c\n> +++ b/builtin/clean.c\n> @@ -262,7 +262,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> \t\t\t\tif (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n> \t\t\t\t\terrors++;\n> \t\t\t\tif (gone && !quiet) {\n> -\t\t\t\t\tqname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n> +\t\t\t\t\tqname = quote_path_relative(directory.buf, prefix, &buf);\n> \t\t\t\t\tprintf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n\nThis one needed a bit closer look to make sure directory.len is\nalready pointing at the end of directory.buf (it is) to verify that\nthis is a safe conversion. Everywhere else we lost either -1 or\nstrlen() of the string we feed quote_path() and quote_path_relative()\nso they look fine.\n"},{"id":"221882","messageId":"7v8v1z8gfu.fsf@alter.siamese.dyndns.org","threadId":"34120","inReplyTo":"6e5f34b62ff6a7d6b379a5cabc07b849539fb08f.1372087065.git.worldhello.net@gmail.com","subject":"Re: [PATCH v14 05/16] Refactor write_name_quoted_relative, remove unused params","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-24T19:19:33Z","receivedAt":"2013-06-24T19:19:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin writes:\n\n> -static void write_name(const char* name, size_t len)\n> +static void write_name(const char *name)\n> {\n> -\twrite_name_quoted_relative(name, len, prefix, prefix_len, stdout,\n> -\t\t\tline_terminator);\n> +\n> +\t/* turn off prefix, if run with \"--full-name\" */\n> +\twrite_name_quoted_relative(name, prefix_len ? prefix : NULL,\n> +\t\t\t\t stdout, line_terminator);\n\nHmmm....\n\nDoes this mean that ls-files has been broken in 03/16, because\nwrite_name_quoted_relative() was made to ignore prefix_len and\nmeasure the length of prefix with strlen(prefix), and this change\nfixes it?\n"},{"id":"221883","messageId":"7v4ncn8gbc.fsf@alter.siamese.dyndns.org","threadId":"34120","inReplyTo":"37345475f7166717c7e3cbfab7e179605dafabdd.1372087065.git.worldhello.net@gmail.com","subject":"Re: [PATCH v14 06/16] git-clean: refactor git-clean into two phases","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-24T19:22:15Z","receivedAt":"2013-06-24T19:22:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin writes:\n\n> Before introducing interactive git-clean, refactor git-clean operations\n> into two phases:\n>\n> * hold cleaning items in del_list,\n> * and remove them in a separate loop at the end.\n>\n> We will introduce interactive git-clean between the two phases. The\n> interactive git-clean will show what would be done and must confirm\n> before do real cleaning.\n>\n> Signed-off-by: Jiang Xin \n> Signed-off-by: Junio C Hamano \n> ---\n\nLooks sensible.\n"},{"id":"221915","messageId":"CANYiYbE52EtdnoPUW5HYio1SQsKoVjEf5BdxbcTqjsNXCmq92g@mail.gmail.com","threadId":"34120","inReplyTo":"51C89655.4070208@kdbg.org","subject":"Re: [PATCH v14 16/16] test: run testcases with POSIX absolute paths on Windows","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-25T00:40:36Z","receivedAt":"2013-06-25T00:40:36Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/6/25 Johannes Sixt :\n> Thanks for working on this. Did you have an opportunity to test on Windows?\n\nYes, I write this commit on Windows, and have tested it in msysGit.\n\n>> +mingw_path() {\n>> + case $2 in\n>> + NO_MINGW)\n>> + echo \"$1\"\n>> + ;;\n>> + *)\n>> + test-path-utils mingw_path \"$1\"\n>> + ;;\n>> + esac\n>> +}\n>> +\n>> +get_prereq_flag() {\n>> + case $1 in\n>> + POSIX)\n>> + echo $1\n>> + ;;\n>> + *)\n>> + ;;\n>> + esac\n>> +}\n>\n> It took me a while to notice that the token POSIX serves a different\n> purpose than NO_MINGW: POSIX is only needed to skip a test, NO_MINGW is\n> only used to avoid a test-path-utils call. The reason for the complexity\n> is that you put both tokens in the same parameter position in the\n> invocations of the test cases.\n>\n> You use NO_MINGW only twice. I would just skip these two tests, then the\n> parameter position is reserved for the prerequisite, and things stay simple.\n>\n\nI agree. I rewrite the commit log and remove NO_MINGW like below:\n\n-- 8-< --\n\n>From 4875562a849422a3f31d6ccf3c0eb8a6693adf0e Mon Sep 17 00:00:00 2001\nMessage-Id: <4875562a849422a3f31d6ccf3c0eb8a6693adf0e.1372120737.git.worldhello.net@gmail.com>\nFrom: Jiang Xin \nDate: Mon, 24 Jun 2013 21:35:23 +0800\nSubject: [PATCH] test: run testcases with POSIX absolute paths on Windows\n\nSome test cases are skipped on Windows by marking with POSIX prereq.\nThis is because arguments look like absolute paths (such as /a/b)\nfor regular Windows programs (*.exe executables, no bash scripts)\nare changed to Windows paths (like C:/msysgit/a/b).\n\nThere is no cygpath nor equivalent on msysGit, but it is easy to\nwrite one. New subcommand \"mingw_path\" is added in test-path-utils,\nso that we can get the expected absolute paths on Windows. E.g.\n\n COMMAND LINE Linux output Windows output\n ================================== ============ ===============\n test-path-utils mingw_path / / C:/msysgit\n test-path-utils mingw_path /a/b/ /a/b/ C:/msysgit/a/b/\n\nWith this utility, most skipped test cases in t0060 can be turned on\nto be tested correctly on Windows.\n\nSigned-off-by: Jiang Xin \n---\n t/t0060-path-utils.sh | 42 ++++++++++++++++++++++--------------------\n test-path-utils.c | 5 +++++\n 2 files changed, 27 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 4deec..d5f35 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -8,13 +8,15 @@ test_description='Test various path utilities'\n . ./test-lib.sh\n\n norm_path() {\n+\texpected=$(test-path-utils mingw_path \"$2\")\n \ttest_expect_success $3 \"normalize path: $1 => $2\" \\\n-\t\"test \\\"\\$(test-path-utils normalize_path_copy '$1')\\\" = '$2'\"\n+\t\"test \\\"\\$(test-path-utils normalize_path_copy '$1')\\\" = '$expected'\"\n }\n\n relative_path() {\n+\texpected=$(test-path-utils mingw_path \"$3\")\n \ttest_expect_success $4 \"relative path: $1 $2 => $3\" \\\n-\t\"test \\\"\\$(test-path-utils relative_path '$1' '$2')\\\" = '$3'\"\n+\t\"test \\\"\\$(test-path-utils relative_path '$1' '$2')\\\" = '$expected'\"\n }\n\n # On Windows, we are using MSYS's bash, which mangles the paths.\n@@ -39,8 +41,8 @@ ancestor() {\n \t test \\\"\\$actual\\\" = '$expected'\"\n }\n\n-# Absolute path tests must be skipped on Windows because due to path mangling\n-# the test program never sees a POSIX-style absolute path\n+# Some absolute path tests should be skipped on Windows due to path mangling\n+# on POSIX-style absolute paths\n case $(uname -s) in\n *MINGW*)\n \t;;\n@@ -73,10 +75,10 @@ norm_path d1/s1//../s2/../../d2 d2\n norm_path d1/.../d2 d1/.../d2\n norm_path d1/..././../d2 d1/d2\n\n-norm_path / / POSIX\n+norm_path / /\n norm_path // / POSIX\n norm_path /// / POSIX\n-norm_path /. / POSIX\n+norm_path /. /\n norm_path /./ / POSIX\n norm_path /./.. ++failed++ POSIX\n norm_path /../. ++failed++ POSIX\n@@ -84,19 +86,19 @@ norm_path /./../.// ++failed++ POSIX\n norm_path /dir/.. / POSIX\n norm_path /dir/sub/../.. / POSIX\n norm_path /dir/sub/../../.. ++failed++ POSIX\n-norm_path /dir /dir POSIX\n-norm_path /dir// /dir/ POSIX\n-norm_path /./dir /dir POSIX\n-norm_path /dir/. /dir/ POSIX\n-norm_path /dir///./ /dir/ POSIX\n-norm_path /dir//sub/.. /dir/ POSIX\n-norm_path /dir/sub/../ /dir/ POSIX\n+norm_path /dir /dir\n+norm_path /dir// /dir/\n+norm_path /./dir /dir\n+norm_path /dir/. /dir/\n+norm_path /dir///./ /dir/\n+norm_path /dir//sub/.. /dir/\n+norm_path /dir/sub/../ /dir/\n norm_path //dir/sub/../. /dir/ POSIX\n-norm_path /dir/s1/../s2/ /dir/s2/ POSIX\n-norm_path /d1/s1///s2/..//../s3/ /d1/s3/ POSIX\n-norm_path /d1/s1//../s2/../../d2 /d2 POSIX\n-norm_path /d1/.../d2 /d1/.../d2 POSIX\n-norm_path /d1/..././../d2 /d1/d2 POSIX\n+norm_path /dir/s1/../s2/ /dir/s2/\n+norm_path /d1/s1///s2/..//../s3/ /d1/s3/\n+norm_path /d1/s1//../s2/../../d2 /d2\n+norm_path /d1/.../d2 /d1/.../d2\n+norm_path /d1/..././../d2 /d1/d2\n\n ancestor / / -1\n ancestor /foo / 0\n@@ -197,8 +199,8 @@ relative_path /a\t/a/b\t\t../\n relative_path /\t\t/a/b/\t\t../../\n relative_path /a/c\t/a/b/\t\t../c\n relative_path /a/c\t/a/b\t\t../c\n-relative_path /a/b\t\"\"\t/a/b\tPOSIX\n-relative_path /a/b \t\"\"\t/a/b\tPOSIX\n+relative_path /a/b\t\"\"\t/a/b\n+relative_path /a/b \t\"\"\t/a/b\n relative_path \"\"\t/a/b\t\t./\n relative_path \"\"\t\"\"\t./\n relative_path \"\"\t\"\"\t./\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex 95ef4..699ef 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -116,6 +116,11 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n\n+\tif (argc == 3 && !strcmp(argv[1], \"mingw_path\")) {\n+\t\tputs(argv[2]);\n+\t\treturn 0;\n+\t}\n+\n \tif (argc == 4 && !strcmp(argv[1], \"relative_path\")) {\n \t\tstruct strbuf sb = STRBUF_INIT;\n \t\tconst char *abs, *base, *rel;\n-- \n1.8.3.1.756.g41beab0\n"},{"id":"221916","messageId":"CANYiYbEK-vc2h64rfNDL5k5yhRsxQR=8xtNJJQwUXmyVHH4Cng@mail.gmail.com","threadId":"34120","inReplyTo":"7vli5z8ha0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v14 01/16] test: add test cases for relative_path","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-25T01:06:51Z","receivedAt":"2013-06-25T01:06:51Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/6/25 Junio C Hamano :\n> Jiang Xin writes:\n>\n>> diff --git a/test-path-utils.c b/test-path-utils.c\n>> index 0092cb..dcc530 100644\n>> --- a/test-path-utils.c\n>> +++ b/test-path-utils.c\n>> @@ -28,6 +28,19 @@ static int normalize_ceiling_entry(struct string_list_item *item, void *unused)\n>> return 1;\n>> }\n>>\n>> +static void normalize_argv_string(const char **var, const char *input)\n>> +{\n>> + if (!strcmp(input, \"\"))\n>> + *var = NULL;\n>> + else if (!strcmp(input, \"\"))\n>> + *var = \"\";\n>> + else\n>> + *var = input;\n>> +\n>> + if (*var && (**var == '<' || **var == '('))\n>> + die(\"Bad value: %s\\n\", input);\n>> +}\n>> +\n>\n> If you have to munge the input string like this anyway, perhaps you\n> can work around the command line mangling done by Windows bash\n> runtime, perhaps add something like:\n>\n> if (*input == '_')\n> input++;\n>\n> and then protecting the path with the underscore, like so?\n>\n> relative_path _/a/b/c/ _/a/b/ c/\n>\n> Wouldn't that let you avoid having to handle \"POSIX\" prereq for\n> these paths?\n>\n\nIn order to test NULL pointer in t/t0060, I have to write normalize_argv_string\nto convert \"\" to NULL. So that I can write test case in\nt/t0060-path-utils.sh like this:\n\n relative_path \"\" \"\" ./\n\nAnd for the same reason, output would be \"(null)\" for NULL pointer.\nUse \"(null)\" not \"\" for output, because I want to make sure the\nconversion must happen for input and output.\n\nI found it would be nice to wrap input empty string as \"\" and\nwrap output of empty string as \"(empty)\" for the readability of both\ninput and output of test cases.\n\nAdd prefix _ to workaround the absolute path rewritten issue in\nmsysGit is interesting, but these test cases have already been\ntested in Linux, right? Patch 16/16 turns on most of the test cases\nwhich could only run under POSIX previously, and test these test\ncases in Windows way.\n\n-- \nJiang Xin\n"},{"id":"221921","messageId":"7v38s66b3i.fsf@alter.siamese.dyndns.org","threadId":"34120","inReplyTo":"CANYiYbEK-vc2h64rfNDL5k5yhRsxQR=8xtNJJQwUXmyVHH4Cng@mail.gmail.com","subject":"Re: [PATCH v14 01/16] test: add test cases for relative_path","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-25T04:57:53Z","receivedAt":"2013-06-25T04:57:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin writes:\n\n> In order to test NULL pointer in t/t0060, I have to write normalize_argv_string\n> to convert \"\" to NULL....\n\nYes, null and empty I already understand.\n\n> Add prefix _ to workaround the absolute path rewritten issue in\n> msysGit is interesting, but these test cases have already been\n> tested in Linux, right?\n\nThe most important thing is what we want to test in these tests.\nThe special test program is to try running the underlying\nrelative_path() by driving it directly, but the only real caller is\nin setup_work_tree(), where two return values from real_path() is\ncompared. On POSIX systems, we know we are feeding paths that both\nbegin with /. Now, on Windows systems, what do we get back from\nreal_path()? C:\\git\\Documentation? /git/Documentation? If the\nformer, then writing /a/b/c in the test input and letting Windows\nbash convert it to C:\\a\\b\\c before it calls the test-path-utils\nwould feed what we expect to see in the real caller, which would be\na more meaningful test than what I suggested (i.e. to feed _/a/b/c\nto test-path-utils (unmolested by Windows bash), strip _ to feed\n/a/b/c to underlying relative_path()).\n\nUnlike relative_path() used in setup.c:setup_work_tree(), for\npath_relative() from quote.c, IIRC, the expected inputs are both\nfull pathnames within the working tree. A typical question the\ncallers to this function asks is like \"The current directory\nobtained from prefix is the Documentation/ directory and we need to\nshow that compat/mkdir.c is modified, relative to the current\ndirectory. ../compat/mkdir.c is what I want to show.\" So in that\nsense, it does not matter if /a/b/c is given as /a/b/c or C:\\a\\b\\c\nas we do not care the leading common part (either / or C:\\) very\nmuch. On the other hand, the test vector you preoared in the first\ntest that all begin with / may not be very useful to make sure that\nthe function behaves the same way before and after your rewrite.\n"},{"id":"221924","messageId":"CANYiYbErGwRFHk=_d42ffMBAzHVKR=ytyDt5Ki9b4Bp-tFYgjQ@mail.gmail.com","threadId":"34120","inReplyTo":"7v38s66b3i.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v14 01/16] test: add test cases for relative_path","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-25T07:02:48Z","receivedAt":"2013-06-25T07:02:48Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/6/25 Junio C Hamano :\n>> Add prefix _ to workaround the absolute path rewritten issue in\n>> msysGit is interesting, but these test cases have already been\n>> tested in Linux, right?\n>\n> The most important thing is what we want to test in these tests.\n> The special test program is to try running the underlying\n> relative_path() by driving it directly, but the only real caller is\n> in setup_work_tree(), where two return values from real_path() is\n> compared. On POSIX systems, we know we are feeding paths that both\n> begin with /. Now, on Windows systems, what do we get back from\n> real_path()? C:\\git\\Documentation? /git/Documentation?\n>\n\nConfirm that on Windows real_path() returns absolute path in\nWindows style, such as: C:/msysgit/git/.git\n\n> Unlike relative_path() used in setup.c:setup_work_tree(), for\n> path_relative() from quote.c, IIRC, the expected inputs are both\n> full pathnames within the working tree. A typical question the\n> callers to this function asks is like \"The current directory\n> obtained from prefix is the Documentation/ directory and we need to\n> show that compat/mkdir.c is modified, relative to the current\n> directory. ../compat/mkdir.c is what I want to show.\" So in that\n> sense, it does not matter if /a/b/c is given as /a/b/c or C:\\a\\b\\c\n> as we do not care the leading common part (either / or C:\\) very\n> much. On the other hand, the test vector you preoared in the first\n> test that all begin with / may not be very useful to make sure that\n> the function behaves the same way before and after your rewrite.\n>\n\nYes, I should add more test cases without the leading '/'.\n\n-- \nJiang Xin\n"},{"id":"221934","messageId":"CANYiYbGvqHpQpp2UD0nBS5pqeimGE7yDMVYr7q9bGA0hSNNNDw@mail.gmail.com","threadId":"34120","inReplyTo":"7vhagn8gtz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v14 03/16] quote.c: remove path_relative, use relative_path instead","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-25T11:09:58Z","receivedAt":"2013-06-25T11:09:58Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/6/25 Junio C Hamano :\n> Jiang Xin writes:\n>\n>> Since there is an enhanced version of relative_path() in path.c,\n>> remove duplicate counterpart path_relative() in quote.c.\n>\n> There is no nice comparison chart before and after like you had in\n> patch 02/16?\n>\n\nYou mean drawing a table to compare output of path_relative and\nrelative_path?\n\nI will rewrite the commit log for patch 03/16 like the following. Need\nto polish spellings and grammars.\n\nquote.c: substitute path_relative with relative_path\n\nSubstitute the function path_relative in quote.c with the function\nrelative_path. Function relative_path can be treated as an enhanced\nand robust version of path_relative.\n\nOutputs of path_relative and it's replacement (relative_path) are the\nsame for the following cases:\n\n path prefix output of path_relative output of relative_path\n ======== ========= ======================= =======================\n /a/b/c/ /a/b/ c/ c/\n /a/b/c /a/b/ c c\n /a/ /a/b/ ../ ../\n / /a/b/ ../../ ../../\n /a/c /a/b/ ../c ../c\n /x/y /a/b/ ../../x/y ../../x/y\n a/b/c/ a/b/ c/ c/\n a/ a/b/ ../ ../\n x/y a/b/ ../../x/y ../../x/y\n /a/b (empty) /a/b /a/b\n /a/b (null) /a/b /a/b\n a/b (empty) a/b a/b\n a/b (null) a/b a/b\n\nBut if both of the path and the prefix are the same, or the returned\nrelative path should be the current directory, the outputs of both\nfunctions are different. Function relative_path returns \"./\", while\nfunction path_relative returns empty string.\n\n path prefix output of path_relative output of relative_path\n ======== ========= ======================= =======================\n /a/b/ /a/b/ (empty) ./\n a/b/ a/b/ (empty) ./\n (empty) (null) (empty) ./\n (empty) (empty) (empty) ./\n\nBut not panic, the callers of path_relative can handle such cases, or\nnever encounter this issue at all. E.g.\n\n * In function quote_path_relative, if the output of path_relative is\n empty, append \"./\" to it, like:\n\n if (!out->len)\n strbuf_addstr(out, \"./\");\n\n * Another caller is write_name_quoted_relative, which is only used by\n builtin/ls-files.c. git-ls-files only show files, so path of files\n will never be identical with the prefix of a directory.\n\nThe following differences show that path_relative can not handle\nextra slashes properly.\n\n path prefix output of path_relative output of relative_path\n ======== ========= ======================= =======================\n /a//b//c/ //a/b// ../../../../a//b//c/ c/\n a/b//c a//b ../b//c c\n\nAnd if prefix has no trailing slash, path_relative can not work properly\neither. But since prefix always has a trailing slash, so it's not a\nproblem.\n\n path prefix output of path_relative output of relative_path\n ======== ========= ======================= =======================\n /a/b/c/ /a/b b/c/ c/\n /a/b /a/b b ./\n /a/b/ /a/b b/ ./\n /a /a/b/ ../../a ../\n a/b/c/ a/b b/c/ c/\n a/b/ a/b b/ ./\n a a/b ../a ../\n x/y a/b/ ../x/y ../../x/y\n a/c a/b c ../c\n /a/ /a/b (empty) ../\n (empty) /a/b ../../ ./\n\n\n>> void write_name_quoted_relative(const char *name, size_t len,\n>> const char *prefix, size_t prefix_len,\n>> FILE *fp, int terminator)\n>> {\n>> struct strbuf sb = STRBUF_INIT;\n>>\n>> - name = path_relative(name, len, &sb, prefix, prefix_len);\n>> + name = relative_path(name, prefix, &sb);\n>\n> Are we sure nobody calls prefix_len pointing into the middle of\n> string, not at the end of \"prefix\"? This is unsafe for such a\n> caller, and to make sure we catch them, we should remove the\n> now-unused prefix_len parameter from this function.\n>\n\nNext two commits will remove the unused parameters, and\nmake this series of patches easy to review. But indeed this\ncommit has flaws, next two commits are fixes. Should I squash\nthem back?\n\n\n-- \nJiang Xin\n"},{"id":"221935","messageId":"CANYiYbHCJWhm9sVwaOrjZ+K6SYhssFmSt4XEqXWXhkZ=8KCqmQ@mail.gmail.com","threadId":"34120","inReplyTo":"7vd2rb8gmc.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v14 04/16] Refactor quote_path_relative, remove unused params","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-25T11:39:55Z","receivedAt":"2013-06-25T11:39:55Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/6/25 Junio C Hamano :\n> Jiang Xin writes:\n>\n>> After substitute path_relative() in quote.c with relative_path() from\n>> path.c, parameters (such as len and prefix_len) are obsolete in function\n>> quote_path_relative(). Remove unused parameters and change the order of\n>> parameters for quote_path_relative() function.\n>> ...\n>> diff --git a/builtin/clean.c b/builtin/clean.c\n>> index 04e39..f77f95 100644\n>> --- a/builtin/clean.c\n>> +++ b/builtin/clean.c\n>> @@ -262,7 +262,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n>> if (remove_dirs(&directory, prefix, rm_flags, dry_run, quiet, &gone))\n>> errors++;\n>> if (gone && !quiet) {\n>> - qname = quote_path_relative(directory.buf, directory.len, &buf, prefix);\n>> + qname = quote_path_relative(directory.buf, prefix, &buf);\n>> printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);\n>\n> This one needed a bit closer look to make sure directory.len is\n> already pointing at the end of directory.buf (it is) to verify that\n> this is a safe conversion. Everywhere else we lost either -1 or\n> strlen() of the string we feed quote_path() and quote_path_relative()\n> so they look fine.\n\nWhen tracking in \"remove_dirs(&directory, prefix, rm_flags, dry_run,\nquiet, &gone)\",\nthere are some suspect strbuf_setlens, like:\n\n strbuf_setlen(path, len);\n strbuf_addstr(path, e->d_name);\n\nand at the end of remove_dirs, there is:\n\n strbuf_setlen(path, original_len);\n\nSo both the but and len will be restored in the end, and directory.len always\npoints to strlen of directory.buf.\n\nSo it's safe for the following refactor:\n\n- qname =\nquote_path_relative(directory.buf, directory.len, &buf, prefix);\n+ qname =\nquote_path_relative(directory.buf, prefix, &buf);\n\n\n\n-- \nJiang Xin\n"},{"id":"221936","messageId":"CANYiYbHTT=M7B3MWz4BvWf-8KGxJsfqhQfLbObgE2=v+stMx+A@mail.gmail.com","threadId":"34120","inReplyTo":"7v8v1z8gfu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v14 05/16] Refactor write_name_quoted_relative, remove unused params","fromName":"Jiang Xin","fromEmail":"worldhello.net@gmail.com","sentAt":"2013-06-25T11:51:13Z","receivedAt":"2013-06-25T11:51:13Z","isPatch":true,"sender":{"key":"worldhello.net@gmail.com","avatar":"https://avatars.githubusercontent.com/u/183860?v=4"},"body":"2013/6/25 Junio C Hamano :\n> Jiang Xin writes:\n>\n>> -static void write_name(const char* name, size_t len)\n>> +static void write_name(const char *name)\n>> {\n>> - write_name_quoted_relative(name, len, prefix, prefix_len, stdout,\n>> - line_terminator);\n>> +\n>> + /* turn off prefix, if run with \"--full-name\" */\n>> + write_name_quoted_relative(name, prefix_len ? prefix : NULL,\n>> + stdout, line_terminator);\n>\n> Hmmm....\n>\n> Does this mean that ls-files has been broken in 03/16, because\n> write_name_quoted_relative() was made to ignore prefix_len and\n> measure the length of prefix with strlen(prefix), and this change\n> fixes it?\n\nI will rewrite and move this chunk to patch 03/16, but leave others\nin this patch for clearity.\n\n\n-- \n蒋鑫\n\n北京群英汇信息技术有限公司\n邮件: worldhello.net@gmail.com\n网址: http://www.ossxp.com/\n博客: http://www.worldhello.net/\n微博: http://weibo.com/gotgit/\n电话: 18601196889\n"},{"id":"221963","messageId":"7v1u7q3sfz.fsf@alter.siamese.dyndns.org","threadId":"34120","inReplyTo":"CANYiYbGvqHpQpp2UD0nBS5pqeimGE7yDMVYr7q9bGA0hSNNNDw@mail.gmail.com","subject":"Re: [PATCH v14 03/16] quote.c: remove path_relative, use relative_path instead","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-06-25T19:23:44Z","receivedAt":"2013-06-25T19:23:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jiang Xin writes:\n\n> 2013/6/25 Junio C Hamano :\n>> Jiang Xin writes:\n>>\n>>> Since there is an enhanced version of relative_path() in path.c,\n>>> remove duplicate counterpart path_relative() in quote.c.\n>>\n>> There is no nice comparison chart before and after like you had in\n>> patch 02/16?\n>>\n>\n> You mean drawing a table to compare output of path_relative and\n> relative_path?\n\nI was interested in comparison between the behaviour the current\ncallers of path_relative() gets, and the behaviour the same callers\nget from the consolidated helper function after your patch.\n"}]}