[v3,0/1] submodule: correct an incorrectly formatted error message

Message

Kaartic Sivaraam Oct. 23, 2021, 12:57 p.m. UTC

Hi Atharva,

Sorry for the delay in sending this. Got held up with other work.

On 21/09/21 10:17 pm, Atharva Raykar wrote:
>>
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index 414fcb63ea..236da214c6 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -2775,7 +2775,7 @@ struct add_data {
>>   };
>>   #define ADD_DATA_INIT { .depth = -1 }
>>
>> -static void show_fetch_remotes(FILE *output, const char *sm_name, const char *git_dir_path)
>> +static void show_fetch_remotes(struct strbuf *msg, const char *sm_name, const char *git_dir_path)
> 
> I like the change from using a strbuf instead of passing the output
> stream and printing to it. But maybe we should rename this function, now
> that it doesn't really 'show' anything? Probably something like
> 'append_fetch_remotes()'?

That's a good point. I've taken your suggestion into account in this v3.

Find the details of the v3 of this patch below.

Changes since v2:

- Renamed the helper function name to be more appropriate, upon suggestion.

Also, I rebased my local branch over the latest 'master'. So, this should apply
cleanly over 'master'.

For reference, the v2 could be found here:

    https://public-inbox.org/git/[e-mail]/

... and the range-diff against v2 could be found below.

--
Sivaraam


Kaartic Sivaraam (1):
  submodule--helper: fix incorrect newlines in an error message

 builtin/submodule--helper.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

Range-diff against v2:
1:  95cbe38be3 ! 1:  7c4887ccf5 submodule--helper: fix incorrect newlines in an error message
    @@ builtin/submodule--helper.c: struct add_data {
      #define ADD_DATA_INIT { .depth = -1 }
      
     -static void show_fetch_remotes(FILE *output, const char *git_dir_path)
    -+static void show_fetch_remotes(struct strbuf *msg, const char *sm_name, const char *git_dir_path)
    ++static void append_fetch_remotes(struct strbuf *msg, const char *sm_name, const char *git_dir_path)
      {
      	struct child_process cp_remote = CHILD_PROCESS_INIT;
      	struct strbuf sb_remote_out = STRBUF_INIT;
    @@ builtin/submodule--helper.c: static int add_submodule(const struct add_data *add
     +						    "locally with remote(s):\n"),
     +					    add_data->sm_name);
     +
    -+				show_fetch_remotes(&msg, add_data->sm_name,
    -+						   submod_gitdir_path);
    ++				append_fetch_remotes(&msg, add_data->sm_name,
    ++						     submod_gitdir_path);
      				free(submod_gitdir_path);
     -				die(_("If you want to reuse this local git "
     -				      "directory instead of cloning again from\n"

Comments

Junio C Hamano Oct. 24, 2021, 6:05 a.m. UTC | #1

Kaartic Sivaraam <[e-mail]> writes:

> Hi Atharva,
>
> Sorry for the delay in sending this. Got held up with other work.
>
> On 21/09/21 10:17 pm, Atharva Raykar wrote:
>>>
>>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>>> index 414fcb63ea..236da214c6 100644
>>> --- a/builtin/submodule--helper.c
>>> +++ b/builtin/submodule--helper.c
>>> @@ -2775,7 +2775,7 @@ struct add_data {
>>>   };
>>>   #define ADD_DATA_INIT { .depth = -1 }
>>>
>>> -static void show_fetch_remotes(FILE *output, const char *sm_name, const char *git_dir_path)
>>> +static void show_fetch_remotes(struct strbuf *msg, const char *sm_name, const char *git_dir_path)
>> 
>> I like the change from using a strbuf instead of passing the output
>> stream and printing to it. But maybe we should rename this function, now
>> that it doesn't really 'show' anything? Probably something like
>> 'append_fetch_remotes()'?
>
> That's a good point. I've taken your suggestion into account in this v3.
>
> Find the details of the v3 of this patch below.

Looking good.

Let's declare victory and merge it down to 'next' and then to
'master'.

Thanks, both.  Will replace.