Re: [PATCH v3 3/3] git: submodule honor -c credential.* from command line

From: Jacob Keller <jacob.keller@gmail.com>
Date: Feb 26, 2016, 22:19 UTC
Message-ID: <CA+P7+xpArLkv2_3juxMFBct1=31hf-gdaOT43pN9CbpuHaz5AQ@mail.gmail.com>
In-Reply-To: <CAGZ79kZnR3EsESCFdORq84awWnRZZ9A_+mcH_54ULZZJfD8xyQ@mail.gmail.com>

On Fri, Feb 26, 2016 at 11:41 AM, Stefan Beller <sbeller@google.com> wrote:

Show 7 quoted lines

>
> In the future, would you mind to try interdiffs in a cover letter?
>
> (I do git diff HEAD...$(previousSeries) with previousSerier either
> local branch or rather what Junio picked up already. There is also tbdiff,
> which should be better and easier than this work flow)
>

Yes, I should do that. I could just use the reflog for this actually, for my workflow.

Show 9 quoted lines

>
>> +                */
>> +               strbuf_addstr(&quoted, var);
>> +               strbuf_addch(&quoted, '=');
>> +               strbuf_addstr(&quoted, value);
>
> This could be `strbuf_addf("%s=%s", var, value);` (?)
> which then gets quoted below
>

Is there such a thing as sq_quote_buf equivalent of this? If not, should I add one? I think that might be preferable since we'd drop the entire extra strbuf variable.

Show 6 quoted lines

>> +                       git_config_from_parameters(sanitize_submodule_config,
>> +                                                  &sanitized_config);
>> +                       argv_array_pushf(out, "%s=%s", *var, sanitized_config.buf);
>
> like here?
>

Ya, I just wasn't thinking of strbuf_addf. But I think I'd rather have sq_quote_f or something created instead to drop the need of a separate strbuf.

Show 7 quoted lines

>> -                       subsha1=$(clear_local_git_env; cd "$sm_path" &&
>> +                       subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
>>                                 git rev-parse --verify HEAD) ||
>
> While at it, we could discuss if we want to replace the pattern cd
> <somewhere> && git-command
> by `git -C <somewhere> <command>` eventually (not in this patch) ?

Yes, we should use git -C instead, but probably not worth too much effort if our plan is to drop the shell code eventually? Might be worth the cleanup, especially if it lets us avoid a subshell. If we still need the subshell for other non-git commands I would prefer to leave it as "(cd <somewhere> && git command && other commands && ... && )

Thanks, Jake