{"thread":{"id":"14738","subject":"[PATCH 5/5] Add a new test for using a custom merge strategy","startedAt":"2008-07-29T15:24:58Z","lastAt":"2008-07-29T21:25:18Z","messageCount":9,"participants":["Miklos Vajna","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"85481","messageId":"cover.1217344802.git.vmiklos@frugalware.org","threadId":"14738","inReplyTo":"cover.1217207602.git.vmiklos@frugalware.org","subject":"[PATCH 0/5] Allow custom merge strategies, take 3","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-29T15:24:58Z","receivedAt":"2008-07-29T15:24:58Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Hi,\n\nChanges from 'take 2':\n\n- the testcase now makes sure the trees are the same for 'theirs' as\n  well.\n\n- patch 4 and 5 are squashed so there is no compile error after applying\n  only some of the patches. (or when bisecting)\n\n- using FLEX_ARRAY in help.h\n\n- the first two patches are unchanged\n\nMiklos Vajna (5):\n  builtin-help: make is_git_command() usable outside builtin-help\n  builtin-help: make list_commands() a bit more generic\n  builtin-help: make it possible to exclude some commands in\n    list_commands()\n  builtin-merge: allow using a custom strategy\n  Add a new test for using a custom merge strategy\n\n Makefile                |    1 +\n builtin-merge.c         |   32 ++++++++++++++++++++-----\n help.c                  |   57 ++++++++++++++++++++++++-----------------------\n help.h                  |   19 +++++++++++++++\n t/t7606-merge-custom.sh |   46 +++++++++++++++++++++++++++++++++++++\n 5 files changed, 120 insertions(+), 35 deletions(-)\n create mode 100644 help.h\n create mode 100755 t/t7606-merge-custom.sh\n"},{"id":"85478","messageId":"fd19583955e9cea5b78a465d23bc127a51940048.1217344803.git.vmiklos@frugalware.org","threadId":"14738","inReplyTo":"cover.1217344802.git.vmiklos@frugalware.org","subject":"[PATCH 1/5] builtin-help: make is_git_command() usable outside builtin-help","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-29T15:24:59Z","receivedAt":"2008-07-29T15:24:59Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Other builtins may want to check if a given command is a valid git\ncommand or not as well. Additionally add a new parameter that specifies\na custom prefix, so that the \"git-\" prefix is no longer hardwired.\nUseful for example to limit the search for \"git-merge-*\".\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n Makefile |    1 +\n help.c   |   25 ++++++++++++++-----------\n help.h   |    6 ++++++\n 3 files changed, 21 insertions(+), 11 deletions(-)\n create mode 100644 help.h\n\ndiff --git a/Makefile b/Makefile\nindex 798a2f2..351afd7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -355,6 +355,7 @@ LIB_H += git-compat-util.h\n LIB_H += graph.h\n LIB_H += grep.h\n LIB_H += hash.h\n+LIB_H += help.h\n LIB_H += list-objects.h\n LIB_H += ll-merge.h\n LIB_H += log-tree.h\ndiff --git a/help.c b/help.c\nindex 3cb1962..08188f5 100644\n--- a/help.c\n+++ b/help.c\n@@ -418,11 +418,11 @@ static int is_executable(const char *name)\n }\n \n static unsigned int list_commands_in_dir(struct cmdnames *cmds,\n-\t\t\t\t\t const char *path)\n+\t\t\t\t\t const char *path,\n+\t\t\t\t\t const char *prefix)\n {\n \tunsigned int longest = 0;\n-\tconst char *prefix = \"git-\";\n-\tint prefix_len = strlen(prefix);\n+\tint prefix_len;\n \tDIR *dir = opendir(path);\n \tstruct dirent *de;\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -430,6 +430,9 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,\n \n \tif (!dir)\n \t\treturn 0;\n+\tif (!prefix)\n+\t\tprefix = \"git-\";\n+\tprefix_len = strlen(prefix);\n \n \tstrbuf_addf(&buf, \"%s/\", path);\n \tlen = buf.len;\n@@ -460,7 +463,7 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,\n \treturn longest;\n }\n \n-static unsigned int load_command_list(void)\n+static unsigned int load_command_list(const char *prefix)\n {\n \tunsigned int longest = 0;\n \tunsigned int len;\n@@ -469,7 +472,7 @@ static unsigned int load_command_list(void)\n \tconst char *exec_path = git_exec_path();\n \n \tif (exec_path)\n-\t\tlongest = list_commands_in_dir(&main_cmds, exec_path);\n+\t\tlongest = list_commands_in_dir(&main_cmds, exec_path, prefix);\n \n \tif (!env_path) {\n \t\tfprintf(stderr, \"PATH not set\\n\");\n@@ -481,7 +484,7 @@ static unsigned int load_command_list(void)\n \t\tif ((colon = strchr(path, PATH_SEP)))\n \t\t\t*colon = 0;\n \n-\t\tlen = list_commands_in_dir(&other_cmds, path);\n+\t\tlen = list_commands_in_dir(&other_cmds, path, prefix);\n \t\tif (len > longest)\n \t\t\tlongest = len;\n \n@@ -505,7 +508,7 @@ static unsigned int load_command_list(void)\n \n static void list_commands(void)\n {\n-\tunsigned int longest = load_command_list();\n+\tunsigned int longest = load_command_list(NULL);\n \tconst char *exec_path = git_exec_path();\n \n \tif (main_cmds.cnt) {\n@@ -551,9 +554,9 @@ static int is_in_cmdlist(struct cmdnames *c, const char *s)\n \treturn 0;\n }\n \n-static int is_git_command(const char *s)\n+int is_git_command(const char *s, const char *prefix)\n {\n-\tload_command_list();\n+\tload_command_list(prefix);\n \treturn is_in_cmdlist(&main_cmds, s) ||\n \t\tis_in_cmdlist(&other_cmds, s);\n }\n@@ -574,7 +577,7 @@ static const char *cmd_to_page(const char *git_cmd)\n \t\treturn \"git\";\n \telse if (!prefixcmp(git_cmd, \"git\"))\n \t\treturn git_cmd;\n-\telse if (is_git_command(git_cmd))\n+\telse if (is_git_command(git_cmd, NULL))\n \t\treturn prepend(\"git-\", git_cmd);\n \telse\n \t\treturn prepend(\"git\", git_cmd);\n@@ -712,7 +715,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n \t}\n \n \talias = alias_lookup(argv[0]);\n-\tif (alias && !is_git_command(argv[0])) {\n+\tif (alias && !is_git_command(argv[0], NULL)) {\n \t\tprintf(\"`git %s' is aliased to `%s'\\n\", argv[0], alias);\n \t\treturn 0;\n \t}\ndiff --git a/help.h b/help.h\nnew file mode 100644\nindex 0000000..73da8d6\n--- /dev/null\n+++ b/help.h\n@@ -0,0 +1,6 @@\n+#ifndef HELP_H\n+#define HELP_H\n+\n+int is_git_command(const char *s, const char *prefix);\n+\n+#endif /* HELP_H */\n-- \n1.6.0.rc0.14.g95f8.dirty\n"},{"id":"85480","messageId":"5a003a0e20d0942c946680e4eade8e9d19f0036b.1217344803.git.vmiklos@frugalware.org","threadId":"14738","inReplyTo":"fd19583955e9cea5b78a465d23bc127a51940048.1217344803.git.vmiklos@frugalware.org","subject":"[PATCH 2/5] builtin-help: make list_commands() a bit more generic","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-29T15:25:00Z","receivedAt":"2008-07-29T15:25:00Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"That function now takes two paramters to control the prefix of the\nlisted commands, and a second parameter to specify the title of the\ntable. This can be useful for listing not only all git commands, but\nspecific ones, like merge strategies.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n help.c |   18 ++++++++++--------\n help.h |    1 +\n 2 files changed, 11 insertions(+), 8 deletions(-)\n\ndiff --git a/help.c b/help.c\nindex 08188f5..7a42517 100644\n--- a/help.c\n+++ b/help.c\n@@ -506,23 +506,25 @@ static unsigned int load_command_list(const char *prefix)\n \treturn longest;\n }\n \n-static void list_commands(void)\n+void list_commands(const char *prefix, const char *title)\n {\n-\tunsigned int longest = load_command_list(NULL);\n+\tunsigned int longest = load_command_list(prefix);\n \tconst char *exec_path = git_exec_path();\n \n \tif (main_cmds.cnt) {\n-\t\tprintf(\"available git commands in '%s'\\n\", exec_path);\n-\t\tprintf(\"----------------------------\");\n-\t\tmput_char('-', strlen(exec_path));\n+\t\tprintf(\"available %s in '%s'\\n\", title, exec_path);\n+\t\tprintf(\"----------------\");\n+\t\tmput_char('-', strlen(title) + strlen(exec_path));\n \t\tputchar('\\n');\n \t\tpretty_print_string_list(&main_cmds, longest);\n \t\tputchar('\\n');\n \t}\n \n \tif (other_cmds.cnt) {\n-\t\tprintf(\"git commands available from elsewhere on your $PATH\\n\");\n-\t\tprintf(\"---------------------------------------------------\\n\");\n+\t\tprintf(\"%s available from elsewhere on your $PATH\\n\", title);\n+\t\tprintf(\"---------------------------------------\");\n+\t\tmput_char('-', strlen(title));\n+\t\tputchar('\\n');\n \t\tpretty_print_string_list(&other_cmds, longest);\n \t\tputchar('\\n');\n \t}\n@@ -702,7 +704,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n \n \tif (show_all) {\n \t\tprintf(\"usage: %s\\n\\n\", git_usage_string);\n-\t\tlist_commands();\n+\t\tlist_commands(\"git-\", \"git commands\");\n \t\tprintf(\"%s\\n\", git_more_info_string);\n \t\treturn 0;\n \t}\ndiff --git a/help.h b/help.h\nindex 73da8d6..0741662 100644\n--- a/help.h\n+++ b/help.h\n@@ -2,5 +2,6 @@\n #define HELP_H\n \n int is_git_command(const char *s, const char *prefix);\n+void list_commands(const char *prefix, const char *title);\n \n #endif /* HELP_H */\n-- \n1.6.0.rc0.14.g95f8.dirty\n"},{"id":"85479","messageId":"5ad105819efb1c905bd01db3d08eb3422d283b3b.1217344803.git.vmiklos@frugalware.org","threadId":"14738","inReplyTo":"5a003a0e20d0942c946680e4eade8e9d19f0036b.1217344803.git.vmiklos@frugalware.org","subject":"[PATCH 3/5] builtin-help: make it possible to exclude some commands in list_commands()","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-29T15:25:01Z","receivedAt":"2008-07-29T15:25:01Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"The supposed method is to build a list of commands to be excluded using\nadd_cmdname(), then pass the list as the new exclude parameter. If no\nexclude is needed, NULL should be used.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n help.c |   26 +++++++++++---------------\n help.h |   14 +++++++++++++-\n 2 files changed, 24 insertions(+), 16 deletions(-)\n\ndiff --git a/help.c b/help.c\nindex 7a42517..858f76a 100644\n--- a/help.c\n+++ b/help.c\n@@ -9,6 +9,7 @@\n #include \"common-cmds.h\"\n #include \"parse-options.h\"\n #include \"run-command.h\"\n+#include \"help.h\"\n \n static struct man_viewer_list {\n \tstruct man_viewer_list *next;\n@@ -300,18 +301,11 @@ static inline void mput_char(char c, unsigned int num)\n \t\tputchar(c);\n }\n \n-static struct cmdnames {\n-\tint alloc;\n-\tint cnt;\n-\tstruct cmdname {\n-\t\tsize_t len;\n-\t\tchar name[1];\n-\t} **names;\n-} main_cmds, other_cmds;\n+struct cmdnames main_cmds, other_cmds;\n \n-static void add_cmdname(struct cmdnames *cmds, const char *name, int len)\n+void add_cmdname(struct cmdnames *cmds, const char *name, int len)\n {\n-\tstruct cmdname *ent = xmalloc(sizeof(*ent) + len);\n+\tstruct cmdname *ent = xmalloc(sizeof(*ent) + len + 1);\n \n \tent->len = len;\n \tmemcpy(ent->name, name, len);\n@@ -463,7 +457,7 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,\n \treturn longest;\n }\n \n-static unsigned int load_command_list(const char *prefix)\n+static unsigned int load_command_list(const char *prefix, struct cmdnames *exclude)\n {\n \tunsigned int longest = 0;\n \tunsigned int len;\n@@ -502,13 +496,15 @@ static unsigned int load_command_list(const char *prefix)\n \t      sizeof(*other_cmds.names), cmdname_compare);\n \tuniq(&other_cmds);\n \texclude_cmds(&other_cmds, &main_cmds);\n+\tif (exclude)\n+\t\texclude_cmds(&main_cmds, exclude);\n \n \treturn longest;\n }\n \n-void list_commands(const char *prefix, const char *title)\n+void list_commands(const char *prefix, const char *title, struct cmdnames *exclude)\n {\n-\tunsigned int longest = load_command_list(prefix);\n+\tunsigned int longest = load_command_list(prefix, exclude);\n \tconst char *exec_path = git_exec_path();\n \n \tif (main_cmds.cnt) {\n@@ -558,7 +554,7 @@ static int is_in_cmdlist(struct cmdnames *c, const char *s)\n \n int is_git_command(const char *s, const char *prefix)\n {\n-\tload_command_list(prefix);\n+\tload_command_list(prefix, NULL);\n \treturn is_in_cmdlist(&main_cmds, s) ||\n \t\tis_in_cmdlist(&other_cmds, s);\n }\n@@ -704,7 +700,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n \n \tif (show_all) {\n \t\tprintf(\"usage: %s\\n\\n\", git_usage_string);\n-\t\tlist_commands(\"git-\", \"git commands\");\n+\t\tlist_commands(\"git-\", \"git commands\", NULL);\n \t\tprintf(\"%s\\n\", git_more_info_string);\n \t\treturn 0;\n \t}\ndiff --git a/help.h b/help.h\nindex 0741662..3eb8cfb 100644\n--- a/help.h\n+++ b/help.h\n@@ -1,7 +1,19 @@\n #ifndef HELP_H\n #define HELP_H\n \n+struct cmdnames {\n+\tint alloc;\n+\tint cnt;\n+\tstruct cmdname {\n+\t\tsize_t len;\n+\t\tchar name[FLEX_ARRAY];\n+\t} **names;\n+};\n+\n int is_git_command(const char *s, const char *prefix);\n-void list_commands(const char *prefix, const char *title);\n+void list_commands(const char *prefix, const char *title, struct cmdnames *exclude);\n+void add_cmdname(struct cmdnames *cmds, const char *name, int len);\n+\n+extern struct cmdnames main_cmds, other_cmds;\n \n #endif /* HELP_H */\n-- \n1.6.0.rc0.14.g95f8.dirty\n"},{"id":"85482","messageId":"722d66a5a897b694f374aa96bd58aff01a2a5932.1217344803.git.vmiklos@frugalware.org","threadId":"14738","inReplyTo":"5ad105819efb1c905bd01db3d08eb3422d283b3b.1217344803.git.vmiklos@frugalware.org","subject":"[PATCH 4/5] builtin-merge: allow using a custom strategy","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-29T15:25:02Z","receivedAt":"2008-07-29T15:25:02Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Allow using a custom strategy, as long as it's named git-merge-foo. The\nerror handling is now done using is_git_command(). The list of available\nstrategies is now shown by list_commands().\n\nIf an invalid strategy is supplied, like -s foobar, then git-merge would\nlist all git-merge-* commands. This is not perfect, since for example\ngit-merge-index is not a valid strategy.\n\nThese are removed from the output by scanning the list of main commands;\nif the git-merge-foo command is listed in the all_strategy list, then\nit's shown, otherwise excluded. This does not exclude commands somewhere\nelse in the PATH, where custom strategies are expected.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n builtin-merge.c |   32 +++++++++++++++++++++++++-------\n 1 files changed, 25 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex e78fa18..29826b1 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -22,6 +22,7 @@\n #include \"log-tree.h\"\n #include \"color.h\"\n #include \"rerere.h\"\n+#include \"help.h\"\n \n #define DEFAULT_TWOHEAD (1<<0)\n #define DEFAULT_OCTOPUS (1<<1)\n@@ -77,7 +78,7 @@ static int option_parse_message(const struct option *opt,\n static struct strategy *get_strategy(const char *name)\n {\n \tint i;\n-\tstruct strbuf err;\n+\tstruct strategy *ret;\n \n \tif (!name)\n \t\treturn NULL;\n@@ -86,12 +87,29 @@ static struct strategy *get_strategy(const char *name)\n \t\tif (!strcmp(name, all_strategy[i].name))\n \t\t\treturn &all_strategy[i];\n \n-\tstrbuf_init(&err, 0);\n-\tfor (i = 0; i < ARRAY_SIZE(all_strategy); i++)\n-\t\tstrbuf_addf(&err, \" %s\", all_strategy[i].name);\n-\tfprintf(stderr, \"Could not find merge strategy '%s'.\\n\", name);\n-\tfprintf(stderr, \"Available strategies are:%s.\\n\", err.buf);\n-\texit(1);\n+\tif (!is_git_command(name, \"git-merge-\")) {\n+\t\tstruct cmdnames not_strategies;\n+\n+\t\tmemset(&not_strategies, 0, sizeof(struct cmdnames));\n+\t\tfor (i = 0; i < main_cmds.cnt; i++) {\n+\t\t\tint j, found = 0;\n+\t\t\tstruct cmdname *ent = main_cmds.names[i];\n+\t\t\tfor (j = 0; j < ARRAY_SIZE(all_strategy); j++)\n+\t\t\t\tif (!strncmp(ent->name, all_strategy[j].name, ent->len)\n+\t\t\t\t\t\t&& !all_strategy[j].name[ent->len])\n+\t\t\t\t\tfound = 1;\n+\t\t\tif (!found)\n+\t\t\t\tadd_cmdname(&not_strategies, ent->name, ent->len);\n+\t\t}\n+\t\tfprintf(stderr, \"Could not find merge strategy '%s'.\\n\\n\", name);\n+\t\tlist_commands(\"git-merge-\", \"strategies\", &not_strategies);\n+\t\texit(1);\n+\t}\n+\n+\tret = xmalloc(sizeof(struct strategy));\n+\tmemset(ret, 0, sizeof(struct strategy));\n+\tret->name = xstrdup(name);\n+\treturn ret;\n }\n \n static void append_strategy(struct strategy *s)\n-- \n1.6.0.rc0.14.g95f8.dirty\n"},{"id":"85477","messageId":"3c77f8dd8b229bb08b5bc0b6a8efd7359df5ecc9.1217344803.git.vmiklos@frugalware.org","threadId":"14738","inReplyTo":"722d66a5a897b694f374aa96bd58aff01a2a5932.1217344803.git.vmiklos@frugalware.org","subject":"[PATCH 5/5] Add a new test for using a custom merge strategy","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-29T15:25:03Z","receivedAt":"2008-07-29T15:25:03Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Testing is done by creating a simple git-merge-theirs strategy which is\nthe opposite of ours. Using this in real merges is not recommended but\nit's perfect for our testing needs.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n t/t7606-merge-custom.sh |   46 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 46 insertions(+), 0 deletions(-)\n create mode 100755 t/t7606-merge-custom.sh\n\ndiff --git a/t/t7606-merge-custom.sh b/t/t7606-merge-custom.sh\nnew file mode 100755\nindex 0000000..13e8ff5\n--- /dev/null\n+++ b/t/t7606-merge-custom.sh\n@@ -0,0 +1,46 @@\n+#!/bin/sh\n+\n+test_description='git-merge\n+\n+Testing a custom strategy.'\n+\n+. ./test-lib.sh\n+\n+cat > git-merge-theirs << EOF\n+#!/bin/sh\n+eval git read-tree --reset -u \\\\\\$\\$#\n+EOF\n+chmod +x git-merge-theirs\n+PATH=.:$PATH\n+export PATH\n+\n+test_expect_success 'setup' '\n+\techo c0 > c0.c &&\n+\tgit add c0.c &&\n+\tgit commit -m c0 &&\n+\tgit tag c0 &&\n+\techo c1 > c1.c &&\n+\tgit add c1.c &&\n+\tgit commit -m c1 &&\n+\tgit tag c1 &&\n+\tgit reset --hard c0 &&\n+\techo c2 > c2.c &&\n+\tgit add c2.c &&\n+\tgit commit -m c2 &&\n+\tgit tag c2\n+'\n+\n+test_expect_success 'merge c2 with a custom strategy' '\n+\tgit reset --hard c1 &&\n+\tgit merge -s theirs c2 &&\n+\ttest \"$(git rev-parse c1)\" != \"$(git rev-parse HEAD)\" &&\n+\ttest \"$(git rev-parse c1)\" = \"$(git rev-parse HEAD^1)\" &&\n+\ttest \"$(git rev-parse c2)\" = \"$(git rev-parse HEAD^2)\" &&\n+\ttest \"$(git rev-parse c2^{tree})\" = \"$(git rev-parse HEAD^{tree})\" &&\n+\tgit diff --exit-code &&\n+\ttest -f c0.c &&\n+\ttest ! -f c1.c &&\n+\ttest -f c2.c\n+'\n+\n+test_done\n-- \n1.6.0.rc0.14.g95f8.dirty\n"},{"id":"85507","messageId":"7vabg0eamk.fsf@gitster.siamese.dyndns.org","threadId":"14738","inReplyTo":"5ad105819efb1c905bd01db3d08eb3422d283b3b.1217344803.git.vmiklos@frugalware.org","subject":"Re: [PATCH 3/5] builtin-help: make it possible to exclude some commands in list_commands()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-29T19:46:59Z","receivedAt":"2008-07-29T19:46:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> The supposed method is to build a list of commands to be excluded using\n> add_cmdname(), then pass the list as the new exclude parameter. If no\n> exclude is needed, NULL should be used.\n\nYou require that exclude is a sorted list; this should be documented\nsomewhere to avoid future misuses.\n\nThere is no need for adding extra qsort() anywhere in the patchset because\nthe only user you are adding is in the next patch that sends a subset of\nthe commands in main_cmds in the order they are found in the list, and\nmain_cmds is already sorted.\n"},{"id":"85508","messageId":"7v4p68eame.fsf@gitster.siamese.dyndns.org","threadId":"14738","inReplyTo":"722d66a5a897b694f374aa96bd58aff01a2a5932.1217344803.git.vmiklos@frugalware.org","subject":"Re: [PATCH 4/5] builtin-merge: allow using a custom strategy","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-29T19:47:05Z","receivedAt":"2008-07-29T19:47:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> Allow using a custom strategy, as long as it's named git-merge-foo. The\n> error handling is now done using is_git_command(). The list of available\n> strategies is now shown by list_commands().\n>\n> If an invalid strategy is supplied, like -s foobar, then git-merge would\n> list all git-merge-* commands. This is not perfect, since for example\n> git-merge-index is not a valid strategy.\n> ...\n> +\tif (!is_git_command(name, \"git-merge-\")) {\n> +\t\tstruct cmdnames not_strategies;\n> +\n> +\t\tmemset(&not_strategies, 0, sizeof(struct cmdnames));\n> +\t\tfor (i = 0; i < main_cmds.cnt; i++) {\n> +\t\t\tint j, found = 0;\n> +\t\t\tstruct cmdname *ent = main_cmds.names[i];\n> +\t\t\tfor (j = 0; j < ARRAY_SIZE(all_strategy); j++)\n> +\t\t\t\tif (!strncmp(ent->name, all_strategy[j].name, ent->len)\n> +\t\t\t\t\t\t&& !all_strategy[j].name[ent->len])\n> +\t\t\t\t\tfound = 1;\n> +\t\t\tif (!found)\n> +\t\t\t\tadd_cmdname(&not_strategies, ent->name, ent->len);\n> +\t\t}\n\nThis feels overly wasteful.  Granted, this is not a performance critical\ncodepath, but you list all commands that begin with \"git-merge-\" in\nis_git_command(), only to discard it and then iterate over 140+ main_cmds\nlist only to cull the ones whose name do not appear in the strategies\nlist.\n\nPerhaps this shows that changing the function is_git_command() is a wrong\napproach (for one thing, with the custom prefix, it is not about \"Is it a\ngit command\" anymore).  Wouldn't it be easier to read if you did this part\nlike this instead?\n\n * make load_command_list capable of loading into a \"struct cmdnames\" (or\n   pair if you want) supplied by the caller;\n\n * use it to grab all commands whose name begin with \"git-merge-\" here;\n\n * Check if name appears in that list; if it doesn't, you already have the\n   list of commands that could be merge strategy for error reporting.\n\nIf you also update list_commands() not to run load_command_list() itself\nbut take caller-supplied list of commands, then the API would become much\ncleaner.  The caller would not be limited to \"filter with prefix\" anymore.\n\nHmm?\n"},{"id":"85520","messageId":"eb25e1eda6daf7c9b49552fe464b58cb05d99ace.1217366612.git.vmiklos@frugalware.org","threadId":"14738","inReplyTo":"7vabg0eamk.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 3/5] builtin-help: make it possible to exclude some commands in list_commands()","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-07-29T21:25:18Z","receivedAt":"2008-07-29T21:25:18Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"The supposed method is to build a list of commands to be excluded using\nadd_cmdname(), then pass the list as the new exclude parameter. If no\nexclude is needed, NULL should be used.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nOn Tue, Jul 29, 2008 at 12:46:59PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> You require that exclude is a sorted list; this should be documented\n> somewhere to avoid future misuses.\n\nThis version now adds a comment in load_command_list() about this.\n\n help.c |   27 ++++++++++++---------------\n help.h |   14 +++++++++++++-\n 2 files changed, 25 insertions(+), 16 deletions(-)\n\ndiff --git a/help.c b/help.c\nindex 7a42517..26aa3e0 100644\n--- a/help.c\n+++ b/help.c\n@@ -9,6 +9,7 @@\n #include \"common-cmds.h\"\n #include \"parse-options.h\"\n #include \"run-command.h\"\n+#include \"help.h\"\n \n static struct man_viewer_list {\n \tstruct man_viewer_list *next;\n@@ -300,18 +301,11 @@ static inline void mput_char(char c, unsigned int num)\n \t\tputchar(c);\n }\n \n-static struct cmdnames {\n-\tint alloc;\n-\tint cnt;\n-\tstruct cmdname {\n-\t\tsize_t len;\n-\t\tchar name[1];\n-\t} **names;\n-} main_cmds, other_cmds;\n+struct cmdnames main_cmds, other_cmds;\n \n-static void add_cmdname(struct cmdnames *cmds, const char *name, int len)\n+void add_cmdname(struct cmdnames *cmds, const char *name, int len)\n {\n-\tstruct cmdname *ent = xmalloc(sizeof(*ent) + len);\n+\tstruct cmdname *ent = xmalloc(sizeof(*ent) + len + 1);\n \n \tent->len = len;\n \tmemcpy(ent->name, name, len);\n@@ -463,7 +457,7 @@ static unsigned int list_commands_in_dir(struct cmdnames *cmds,\n \treturn longest;\n }\n \n-static unsigned int load_command_list(const char *prefix)\n+static unsigned int load_command_list(const char *prefix, struct cmdnames *exclude)\n {\n \tunsigned int longest = 0;\n \tunsigned int len;\n@@ -502,13 +496,16 @@ static unsigned int load_command_list(const char *prefix)\n \t      sizeof(*other_cmds.names), cmdname_compare);\n \tuniq(&other_cmds);\n \texclude_cmds(&other_cmds, &main_cmds);\n+\tif (exclude)\n+\t\t/* Here we require that exclude is a sorted list. */\n+\t\texclude_cmds(&main_cmds, exclude);\n \n \treturn longest;\n }\n \n-void list_commands(const char *prefix, const char *title)\n+void list_commands(const char *prefix, const char *title, struct cmdnames *exclude)\n {\n-\tunsigned int longest = load_command_list(prefix);\n+\tunsigned int longest = load_command_list(prefix, exclude);\n \tconst char *exec_path = git_exec_path();\n \n \tif (main_cmds.cnt) {\n@@ -558,7 +555,7 @@ static int is_in_cmdlist(struct cmdnames *c, const char *s)\n \n int is_git_command(const char *s, const char *prefix)\n {\n-\tload_command_list(prefix);\n+\tload_command_list(prefix, NULL);\n \treturn is_in_cmdlist(&main_cmds, s) ||\n \t\tis_in_cmdlist(&other_cmds, s);\n }\n@@ -704,7 +701,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)\n \n \tif (show_all) {\n \t\tprintf(\"usage: %s\\n\\n\", git_usage_string);\n-\t\tlist_commands(\"git-\", \"git commands\");\n+\t\tlist_commands(\"git-\", \"git commands\", NULL);\n \t\tprintf(\"%s\\n\", git_more_info_string);\n \t\treturn 0;\n \t}\ndiff --git a/help.h b/help.h\nindex 0741662..3eb8cfb 100644\n--- a/help.h\n+++ b/help.h\n@@ -1,7 +1,19 @@\n #ifndef HELP_H\n #define HELP_H\n \n+struct cmdnames {\n+\tint alloc;\n+\tint cnt;\n+\tstruct cmdname {\n+\t\tsize_t len;\n+\t\tchar name[FLEX_ARRAY];\n+\t} **names;\n+};\n+\n int is_git_command(const char *s, const char *prefix);\n-void list_commands(const char *prefix, const char *title);\n+void list_commands(const char *prefix, const char *title, struct cmdnames *exclude);\n+void add_cmdname(struct cmdnames *cmds, const char *name, int len);\n+\n+extern struct cmdnames main_cmds, other_cmds;\n \n #endif /* HELP_H */\n-- \n1.6.0.rc0.14.g95f8.dirty\n"}]}