Re: [PATCH 1/2] [Outreachy] gpg-interface: replace strbuf_split with string_list_split

>
> On Wed, Oct 15, 2025 at 4:19 AM Olamide Caleb Bello <belkid98@gmail.com> wrote:
> >
> > get_ssh_finger_print() accepts a signing key and then uses pipe_command
>
> "pipe_command" is a function too so it's better to use "()" when
> talking about it like in "get_ssh_finger_print()".
>
> In the commit message subject, if it doesn't make it too long, I think
> it might be better to also use "()" when talking about functions.
>
> > to execute the ssh-keygen command, gets its output and sets it in
> > fingerprint_stdout.
>
> Anyway I am not sure we need so many details about what
> get_ssh_finger_print() does before the split.
>
> Maybe saying something like the following is enough:
>
> "In get_ssh_finger_print(), the output of the `ssh-keygen` command is
> put into `fingerprint_stdout`."
>
> > The string in fingerprint_stdout is then split into 3 strbufs using
> > strbuf_split_max(), however they are not modified after the split thereby
> > not making use of the strbuf API as the fingerprint token is merely
> > returned, hence they do not need to be strbufs.
>
> It might be interesting to say that the fingerprint token is returned
> as a `char *` not a strbuf.
>
> > Use string_list_split instead for simplicity.
>
> Here also using "()" could make it clearer that "string_list_split" is
> a function.
>
> > Note that strbuf_split_max uses 3 to specify the number of tokens to
>
> Here also using "()" could help a bit.
>
> > extract from the string, while string_list_split uses 2 because it specifies
>
> Here also using "()" could help a bit.
>
> > the number of times the split will be done on the string, so 2 gives 3 tokens
> > as it is in the initial instance.
>
> Maybe: s/initial/original/

>
> > Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
>
> [...]
>
> > @@ -845,13 +846,12 @@ static char *get_ssh_key_fingerprint(const char *signing_key)
> >                 die_errno(_("failed to get the ssh fingerprint for key '%s'"),
> >                           signing_key);
> >
> > -       fingerprint = strbuf_split_max(&fingerprint_stdout, ' ', 3);
> > -       if (!fingerprint[1])
> > +       if (string_list_split(&split, fingerprint_stdout.buf, " ", 2) <= 1)
>
> According to its doc, string_list_split() returns the the number of
> substrings appended to the list. And you said in the commit message
> that it should give 3 tokens, so I think the above line should be:
>
> if (string_list_split(&split, fingerprint_stdout.buf, " ", 2) < 3)
>
> or even:
>
> if (string_list_split(&split, fingerprint_stdout.buf, " ", 2) != 3)
>