Re: [PATCH 02/14] pull: pass verbosity, --progress flags to fetch and merge

Hi Paul,

On 2015-05-21 11:48, Paul Tan wrote:

On Tue, May 19, 2015 at 1:41 AM, Johannes Schindelin
[off-list ref] wrote:

quoted

On 2015-05-18 17:05, Paul Tan wrote:

quoted

diff --git a/builtin/pull.c b/builtin/pull.c
index 0b771b9..a4d9c92 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c

@@ -11,16 +11,64 @@
 #include "argv-array.h"
 #include "run-command.h"

+/**
+ * Given an option opt, where opt->value points to a char* and opt->defval is a
+ * string, sets opt->value to the evaluation of "--$defval=$arg". If `arg` is
+ * NULL, then opt->value is set to "--$defval". If unset is true, then
+ * opt->value is set to "--no-$defval".
+ */
+static int parse_opt_passthru(const struct option *opt, const char

*arg, int unset)

Implied by my suggested description, I also propose to put the re-recreated command-line option into a strbuf instead of a char *, to make memory management easier (read: avoid memory leaks).

Unfortunately, the usage of strbuf means that we lose the ability to
know if an option was not provided at all (the value is NULL). This is
important as some of these options are used to override the default
configured behavior.

Would this not be implied by the strbuf's len being 0?

quoted

You might also want to verify that arg is `NULL` when `unset != 0`. Something like this:

Hmm, would there be a situation where arg is NULL when `unset != 0`?

I forgot to say that my suggestion was purely meant as defensive coding. Yes, `arg` *should* be `NULL` when `unset != 0`. Should ;-)

By the way, just to make sure: My comments and suggestions are no demands; you should feel very free to reject them when you feel that your code is better without the suggested changes. I am just trying to be helpful.

Ciao,
Dscho