threads / patch / 38703

patchAdds configuration options for some commonly used command-line options (GSOC micro-project)

Subject: [PATCH] Adds configuration options for some commonly used command-line options (GSOC micro-project)

## tl;dr

2 messages between Mar 4, 2015 and Mar 5, 2015. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Amate Yolande· Mar 4, 2015, 23:59 UTC · lore
	This is a patch for my work on one of the micro projects, as I intend
to apply for the Google Summer of Code 2015 under the Git community.
This patch gives the user the oppotunity to specify configuration
options for some commonly used command-line options for exampel:
	git config defopt.am 'am -3'
---
 Makefile |    1 +
 defopt.c |   24 ++++++++++++++++++++++++
 git.c    |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+)
 create mode 100644 defopt.c
Show changes to 3 files +81 −0

Makefile, defopt.c, git.c

diff --git a/Makefile b/Makefile
index 2320de5..e713e23 100644
--- a/Makefile
+++ b/Makefile
@@ -789,6 +789,7 @@ LIB_OBJS += csum-file.o
 LIB_OBJS += ctype.o
 LIB_OBJS += date.o
 LIB_OBJS += decorate.o
+LIB_OBJS += defopt.o
 LIB_OBJS += diffcore-break.o
 LIB_OBJS += diffcore-delta.o
 LIB_OBJS += diffcore-order.o
diff --git a/defopt.c b/defopt.c
new file mode 100644
index 0000000..b4fa9e2
--- /dev/null
+++ b/defopt.c
@@ -0,0 +1,24 @@
+#include "cache.h"
+
+static const char *defopt_key;
+static char *defopt_val;
+
+static int defopt_lookup_cb(const char *k, const char *v, void *cb)
+{
+	const char *name;
+	if (skip_prefix(k, "defopt.", &name) && !strcmp(name, defopt_key)) {
+		if (!v)
+			return config_error_nonbool(k);
+		defopt_val = xstrdup(v);
+		return 0;
+	}
+	return 0;
+}
+
+char *defopt_lookup(const char *defopt)
+{
+	defopt_key = defopt;
+	defopt_val = NULL;
+	git_config(defopt_lookup_cb, NULL);
+	return defopt_val;
+}
diff --git a/git.c b/git.c
index 9c49519..4b556e1 100644
--- a/git.c
+++ b/git.c
@@ -223,6 +223,59 @@ static int handle_options(const char ***argv, int
*argc, int *envchanged)
 	return (*argv) - orig_argv;
 }

+static int handle_defopt(int *argcp, const char ***argv)
+{	
+	int envchanged = 0, ret = 0, saved_errno = errno;
+	const char *subdir;
+	int count, option_count;
+	const char **new_argv;
+	const char *defopt_command;
+	char *defopt_string;
+	int unused_nongit;
+
+	subdir = setup_git_directory_gently(&unused_nongit);
+
+	defopt_command = (*argv)[0];
+	defopt_string = defopt_lookup(defopt_command);
+	if (defopt_string) {
+		
+		count = split_cmdline(defopt_string, &new_argv);
+		if (count < 0)
+			return;
+		option_count = handle_options(&new_argv, &count, &envchanged);
+		
+		memmove(new_argv - option_count, new_argv,
+				count * sizeof(char *));
+		new_argv -= option_count;
+
+		if (count < 1)
+			return;
+
+		if (strcmp(defopt_command, new_argv[0]))
+			return;
+
+		trace_argv_printf(new_argv,
+				  "trace: defopt: %s =>",
+				  defopt_command);
+
+		new_argv = xrealloc(new_argv, sizeof(char *) *
+				    (count + *argcp));
+		/* insert after command name */
+		memcpy(new_argv + count, *argv + 1, sizeof(char *) * *argcp);
+
+		*argv = new_argv;
+		*argcp += count - 1;
+
+	}
+
+	if (subdir && chdir(subdir))
+		die_errno("Cannot change to '%s'", subdir);
+
+	errno = saved_errno;
+
+}
+
+
 static int handle_alias(int *argcp, const char ***argv)
 {
 	int envchanged = 0, ret = 0, saved_errno = errno;
@@ -517,6 +570,9 @@ static void handle_builtin(int argc, const char **argv)
 		argv[1] = argv[0];
 		argv[0] = cmd = "help";
 	}
+	
+	if(is_builtin(cmd) && argc == 1)
+		handle_defopt(&argc, &argv);

 	for (i = 0; i < ARRAY_SIZE(commands); i++) {
 		struct cmd_struct *p = commands+i;
-- 
	Signed-off-by: Yolande Amate <yolandeamate@gmail.com>
1.7.10.4
Junio C Hamano· Mar 5, 2015, 01:11 UTC · re: Amate Yolande · lore

Re: [PATCH] Adds configuration options for some commonly used command-line options (GSOC micro-project)

Amate Yolande <yolandeamate@gmail.com> writes:
Show 6 quoted lines
> 	This is a patch for my work on one of the micro projects, as I intend
> to apply for the Google Summer of Code 2015 under the Git community.
> This patch gives the user the oppotunity to specify configuration
> options for some commonly used command-line options for exampel:
> 	git config defopt.am 'am -3'
> ---

Check Documentaiton/SubmittingPatches again? It would be beneficial to use the output of "git log --no-merges" for recent history to see the recommended style of log messages while studying it.

>  Makefile |    1 +
>  defopt.c |   24 ++++++++++++++++++++++++
>  git.c    |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Docs and tests?
> +static int handle_defopt(int *argcp, const char ***argv)
> +{	
> +	int envchanged = 0, ret = 0, saved_errno = errno;

What is the point of having a local envchanged here, receiving the info from handle_options() only to discard?

> +	subdir = setup_git_directory_gently(&unused_nongit);
> + ...
> +	if (subdir && chdir(subdir))
> +		die_errno("Cannot change to '%s'", subdir);

Why do you need to do this chdir() here? Wouldn't the caller of the codepath after the callsite you added the call to this function go to the top-level as necessary already?

Show 15 quoted lines
> +	errno = saved_errno;
> +
> +}
> +
> +
>  static int handle_alias(int *argcp, const char ***argv)
>  {
>  	int envchanged = 0, ret = 0, saved_errno = errno;
> @@ -517,6 +570,9 @@ static void handle_builtin(int argc, const char **argv)
>  		argv[1] = argv[0];
>  		argv[0] = cmd = "help";
>  	}
> +	
> +	if(is_builtin(cmd) && argc == 1)
> +		handle_defopt(&argc, &argv);
You used "am -3" as an example, but is "am" a built-in?

Even if it were, being able to say "git am" (no arguments) and getting that rewritten to "git am -3", only when there is no other arguments, is not all that useful, as a typical workflow with "am" is to save a series of patches in a mailbox file (e.g. I would save the message I am responding to in ./+ay-defopt.mbox) and then feed that as an argument (e.g. "git am ./+ay-defopt.mbox").

A lazier version of me (but not me who is typing this message) might appreciate it if "git am ./+ay-defopt.mbox" gets rewritten to "git am -3 ./+ay-defopt.mbox" by setting "git config am.threeway yes" once, while having an option to countermand that per invocation, by saying "git am --no-3way ./+ay-defopt.mbox".

I think what I am saying is that an ultra-generic solution like the patch I am commenting on implements is way too simple to be useful.

With today's code, our users can do this once:
    git config alias.am3 "am -3"

and then "git am3 ./+ay-defopt.mbox" would behave as if they typed "git am -3 ./+ay-defopt.mbox", which would already be one step more useful than this "only when there is no argument" design.

I think the problem with this patch ultimately came from a poor phrasing of the Micro suggestion, which said something like "find some common command options and add configuration". It was meant to allow many different people to do many different things (i.e. one person does am.threeway and am.threeway only), not an invitation to design something that is generic that covers all these command options in one go.

So, perhaps limit the scope of Micro to a single command with a few commonly desired configured options and try again?

Thanks.

← back to recent threads