{"thread":{"id":"38703","subject":"[PATCH] Adds configuration options for some commonly used command-line options (GSOC micro-project)","startedAt":"2015-03-04T23:59:48Z","lastAt":"2015-03-05T01:11:38Z","messageCount":2,"participants":["Amate Yolande","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"257036","messageId":"CAFAMDXbY6szmQ-gLB0_j2cYRVHCsJfszw2XU16eb6i5NJPX_pQ@mail.gmail.com","threadId":"38703","inReplyTo":null,"subject":"[PATCH] Adds configuration options for some commonly used command-line options (GSOC micro-project)","fromName":"Amate Yolande","fromEmail":"yolandeamate@gmail.com","sentAt":"2015-03-04T23:59:48Z","receivedAt":"2015-03-04T23:59:48Z","isPatch":true,"sender":{"key":"yolandeamate@gmail.com","avatar":null},"body":"\tThis is a patch for my work on one of the micro projects, as I intend\nto apply for the Google Summer of Code 2015 under the Git community.\nThis patch gives the user the oppotunity to specify configuration\noptions for some commonly used command-line options for exampel:\n\tgit config defopt.am 'am -3'\n---\n Makefile |    1 +\n defopt.c |   24 ++++++++++++++++++++++++\n git.c    |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 81 insertions(+)\n create mode 100644 defopt.c\n\ndiff --git a/Makefile b/Makefile\nindex 2320de5..e713e23 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -789,6 +789,7 @@ LIB_OBJS += csum-file.o\n LIB_OBJS += ctype.o\n LIB_OBJS += date.o\n LIB_OBJS += decorate.o\n+LIB_OBJS += defopt.o\n LIB_OBJS += diffcore-break.o\n LIB_OBJS += diffcore-delta.o\n LIB_OBJS += diffcore-order.o\ndiff --git a/defopt.c b/defopt.c\nnew file mode 100644\nindex 0000000..b4fa9e2\n--- /dev/null\n+++ b/defopt.c\n@@ -0,0 +1,24 @@\n+#include \"cache.h\"\n+\n+static const char *defopt_key;\n+static char *defopt_val;\n+\n+static int defopt_lookup_cb(const char *k, const char *v, void *cb)\n+{\n+\tconst char *name;\n+\tif (skip_prefix(k, \"defopt.\", &name) && !strcmp(name, defopt_key)) {\n+\t\tif (!v)\n+\t\t\treturn config_error_nonbool(k);\n+\t\tdefopt_val = xstrdup(v);\n+\t\treturn 0;\n+\t}\n+\treturn 0;\n+}\n+\n+char *defopt_lookup(const char *defopt)\n+{\n+\tdefopt_key = defopt;\n+\tdefopt_val = NULL;\n+\tgit_config(defopt_lookup_cb, NULL);\n+\treturn defopt_val;\n+}\ndiff --git a/git.c b/git.c\nindex 9c49519..4b556e1 100644\n--- a/git.c\n+++ b/git.c\n@@ -223,6 +223,59 @@ static int handle_options(const char ***argv, int\n*argc, int *envchanged)\n \treturn (*argv) - orig_argv;\n }\n\n+static int handle_defopt(int *argcp, const char ***argv)\n+{\t\n+\tint envchanged = 0, ret = 0, saved_errno = errno;\n+\tconst char *subdir;\n+\tint count, option_count;\n+\tconst char **new_argv;\n+\tconst char *defopt_command;\n+\tchar *defopt_string;\n+\tint unused_nongit;\n+\n+\tsubdir = setup_git_directory_gently(&unused_nongit);\n+\n+\tdefopt_command = (*argv)[0];\n+\tdefopt_string = defopt_lookup(defopt_command);\n+\tif (defopt_string) {\n+\t\t\n+\t\tcount = split_cmdline(defopt_string, &new_argv);\n+\t\tif (count < 0)\n+\t\t\treturn;\n+\t\toption_count = handle_options(&new_argv, &count, &envchanged);\n+\t\t\n+\t\tmemmove(new_argv - option_count, new_argv,\n+\t\t\t\tcount * sizeof(char *));\n+\t\tnew_argv -= option_count;\n+\n+\t\tif (count < 1)\n+\t\t\treturn;\n+\n+\t\tif (strcmp(defopt_command, new_argv[0]))\n+\t\t\treturn;\n+\n+\t\ttrace_argv_printf(new_argv,\n+\t\t\t\t  \"trace: defopt: %s =>\",\n+\t\t\t\t  defopt_command);\n+\n+\t\tnew_argv = xrealloc(new_argv, sizeof(char *) *\n+\t\t\t\t    (count + *argcp));\n+\t\t/* insert after command name */\n+\t\tmemcpy(new_argv + count, *argv + 1, sizeof(char *) * *argcp);\n+\n+\t\t*argv = new_argv;\n+\t\t*argcp += count - 1;\n+\n+\t}\n+\n+\tif (subdir && chdir(subdir))\n+\t\tdie_errno(\"Cannot change to '%s'\", subdir);\n+\n+\terrno = saved_errno;\n+\n+}\n+\n+\n static int handle_alias(int *argcp, const char ***argv)\n {\n \tint envchanged = 0, ret = 0, saved_errno = errno;\n@@ -517,6 +570,9 @@ static void handle_builtin(int argc, const char **argv)\n \t\targv[1] = argv[0];\n \t\targv[0] = cmd = \"help\";\n \t}\n+\t\n+\tif(is_builtin(cmd) && argc == 1)\n+\t\thandle_defopt(&argc, &argv);\n\n \tfor (i = 0; i < ARRAY_SIZE(commands); i++) {\n \t\tstruct cmd_struct *p = commands+i;\n-- \n\tSigned-off-by: Yolande Amate <yolandeamate@gmail.com>\n1.7.10.4\n"},{"id":"257045","messageId":"xmqqvbig5jud.fsf@gitster.dls.corp.google.com","threadId":"38703","inReplyTo":"CAFAMDXbY6szmQ-gLB0_j2cYRVHCsJfszw2XU16eb6i5NJPX_pQ@mail.gmail.com","subject":"Re: [PATCH] Adds configuration options for some commonly used command-line options (GSOC micro-project)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-05T01:11:38Z","receivedAt":"2015-03-05T01:11:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Amate Yolande <yolandeamate@gmail.com> writes:\n\n> \tThis is a patch for my work on one of the micro projects, as I intend\n> to apply for the Google Summer of Code 2015 under the Git community.\n> This patch gives the user the oppotunity to specify configuration\n> options for some commonly used command-line options for exampel:\n> \tgit config defopt.am 'am -3'\n> ---\n\nCheck Documentaiton/SubmittingPatches again?  It would be beneficial\nto use the output of \"git log --no-merges\" for recent history to see\nthe recommended style of log messages while studying it.\n\n>  Makefile |    1 +\n>  defopt.c |   24 ++++++++++++++++++++++++\n>  git.c    |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n\nDocs and tests?\n\n> +static int handle_defopt(int *argcp, const char ***argv)\n> +{\t\n> +\tint envchanged = 0, ret = 0, saved_errno = errno;\n\nWhat is the point of having a local envchanged here, receiving the\ninfo from handle_options() only to discard?\n\n> +\tsubdir = setup_git_directory_gently(&unused_nongit);\n> + ...\n> +\tif (subdir && chdir(subdir))\n> +\t\tdie_errno(\"Cannot change to '%s'\", subdir);\n\nWhy do you need to do this chdir() here?  Wouldn't the caller of the\ncodepath after the callsite you added the call to this function go\nto the top-level as necessary already?\n\n> +\terrno = saved_errno;\n> +\n> +}\n> +\n> +\n>  static int handle_alias(int *argcp, const char ***argv)\n>  {\n>  \tint envchanged = 0, ret = 0, saved_errno = errno;\n> @@ -517,6 +570,9 @@ static void handle_builtin(int argc, const char **argv)\n>  \t\targv[1] = argv[0];\n>  \t\targv[0] = cmd = \"help\";\n>  \t}\n> +\t\n> +\tif(is_builtin(cmd) && argc == 1)\n> +\t\thandle_defopt(&argc, &argv);\n\nYou used \"am -3\" as an example, but is \"am\" a built-in?\n\nEven if it were, being able to say \"git am\" (no arguments) and\ngetting that rewritten to \"git am -3\", only when there is no other\narguments, is not all that useful, as a typical workflow with \"am\"\nis to save a series of patches in a mailbox file (e.g. I would save\nthe message I am responding to in ./+ay-defopt.mbox) and then feed\nthat as an argument (e.g. \"git am ./+ay-defopt.mbox\").\n\nA lazier version of me (but not me who is typing this message) might\nappreciate it if \"git am ./+ay-defopt.mbox\" gets rewritten to \"git\nam -3 ./+ay-defopt.mbox\" by setting \"git config am.threeway yes\"\nonce, while having an option to countermand that per invocation, by\nsaying \"git am --no-3way ./+ay-defopt.mbox\".\n\nI think what I am saying is that an ultra-generic solution like the\npatch I am commenting on implements is way too simple to be useful.\n\nWith today's code, our users can do this once:\n\n    git config alias.am3 \"am -3\"\n\nand then \"git am3 ./+ay-defopt.mbox\" would behave as if they typed\n\"git am -3 ./+ay-defopt.mbox\", which would already be one step more\nuseful than this \"only when there is no argument\" design.\n\nI think the problem with this patch ultimately came from a poor\nphrasing of the Micro suggestion, which said something like \"find\nsome common command options and add configuration\".  It was meant to\nallow many different people to do many different things (i.e. one\nperson does am.threeway and am.threeway only), not an invitation to\ndesign something that is generic that covers all these command\noptions in one go.\n\nSo, perhaps limit the scope of Micro to a single command with a few\ncommonly desired configured options and try again?\n\nThanks.\n"}]}