Re: [PATCH v3 0/3] submodule: port subcommand add from shell to C

[Junio C Hamano <gitster@pobox.com>]

Shourya Shukla <periperidip@gmail.com> writes:

>     3. In the following segment:
>         /*
>          * NEEDSWORK: In a multi-working-tree world, this needs to be
>          * set in the per-worktree config.
>          */
>         if (!git_config_get_string("submodule.active", &var) && var) {
>
>         There was a comment: "What if this were a valueless true
>         ("[submodule] active\n" without "= true")?  Wouldn't get_string()
>         fail?"
>
>         I was under the impression that even if the above failed, it
>         will not really affect the big picture since at the we will set
>         'submodule.name.active" as true irrespective of the above value.
>         Is this correct?

Let's see what kind of value the "submodule.active" variable is
meant to be set to.  Documentation/config/submodule.txt has this:

    submodule.active::
            A repeated field which contains a pathspec used to match against a
            submodule's path to determine if the submodule is of interest to git
            commands. See linkgit:gitsubmodules[7] for details.

It definitely is a string value, and making it a valueless true is
an error in the configuration.  I wonder if we want to diagnose such
an error, or can we just pretend we didn't see it and keep going?

Also the "var" (one of the values set for this multi-valued
variable) is never used in the body of the "if" statement.  The
other user of "submodule.active" in module_init() seems to use
config_get_value_multi() on it.  The new code may deserve a comment
to explain why that is OK to (1) grab just a single value out of the
multi-valued variable, and (2) not even look at its value.