Re: [PATCH v2] ssh signing: better error message when key not in agent

> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1270/radicle-dev/maint-v2
> > Pull-Request: https://github.com/git/git/pull/1270
> >
> > Range-diff vs v1:
> >
> >  1:  0ce06076242 < -:  ----------- ssh signing: better error message when key not in agent
> >  -:  ----------- > 1:  03dfca79387 ssh signing: better error message when key not in agent
>
> This is a fairly useless range-diff.
>
> Even when a range-diff shows the differences in the patches,
> mechanically generated range-diff can only show _what_ changed.  It
> is helpful to explain the changes in your own words to highlight
> _why_ such changes are done, and this place after the "---" line
> and the diffstat we see below is the place to do so.
>
> Does GitGitGadget allow its users to describe the differences since
> the previous iteration yourself?

> >  gpg-interface.c | 15 ++++++++++-----
> >  1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/gpg-interface.c b/gpg-interface.c
> > index f877a1ea564..33899a450eb 100644
> > --- a/gpg-interface.c
> > +++ b/gpg-interface.c
> > @@ -998,6 +998,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
> >       char *ssh_signing_key_file = NULL;
> >       struct strbuf ssh_signature_filename = STRBUF_INIT;
> >       const char *literal_key = NULL;
> > +     int literal_ssh_key = 0;
> >
> >       if (!signing_key || signing_key[0] == '\0')
> >               return error(
> > @@ -1005,6 +1006,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
> >
> >       if (is_literal_ssh_key(signing_key, &literal_key)) {
> >               /* A literal ssh key */
> > +             literal_ssh_key = 1;
> >               key_file = mks_tempfile_t(".git_signing_key_tmpXXXXXX");
> >               if (!key_file)
> >                       return error_errno(
> > @@ -1036,11 +1038,14 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
> >       }
> >
> >       strvec_pushl(&signer.args, use_format->program,
> > -                  "-Y", "sign",
> > -                  "-n", "git",
> > -                  "-f", ssh_signing_key_file,
> > -                  buffer_file->filename.buf,
> > -                  NULL);
> > +                     "-Y", "sign",
> > +                     "-n", "git",
> > +                     "-f", ssh_signing_key_file,
> > +                     NULL);
>
> Please avoid making a pointless indentation change like this.

> We do
> not pass filename yet with this pushl(), because ...
>
> > +     if (literal_ssh_key) {
> > +             strvec_push(&signer.args, "-U");
> > +     }
>
> ... when we give a literal key, we want to insert "-U" in front, and then
>
> > +     strvec_push(&signer.args, buffer_file->filename.buf);
>
> ... the filename.  Which makes sense.