Re: [RFC PATCH] credential: minor documentation fixes

["Carlo Marcelo Arenas Belón" <carenas@gmail.com>]

On Mon, May 04, 2020 at 10:44:36AM -0400, Jeff King wrote:
> On Mon, May 04, 2020 at 12:45:20AM -0700, Carlo Marcelo Arenas Belón wrote:
> > * clarification on which fields can be updated by the helper; currently I
> >   don't think we allow overrides for protocol and host and assume all others
> >   but the documentation doesn't clarify, and that might be a problem for
> >   cert:// where file is more relevant.
> 
> I think we do allow a helper to transform a credential in any way:
> 
>   echo url=https://example.com/ |
>   git \
>     -c credential.helper='!sed >&2 s/^/one:/; echo host=other.example.com;:' \
>     -c credential.helper='!sed >&2 s/^/two:/;:' \
>     credential fill
> 
> produces:
> 
>   one:protocol=https
>   one:host=example.com
>   two:protocol=https
>   two:host=other.example.com
>   Username for 'https://other.example.com': 
> 
> So after the first helper runs, subsequent helpers (and the internal
> terminal prompt) will consider the modified hostname.

correct, and that is because the specification (and the current code) does
say that is a valid operation.

what I meant is that we don't REALLY allow those modification to take place
since the credential helper suggestions can't really affect what git assumes
is the right host or protocol (or FWIW path), unlike username/password  and
so they don't seem worth having IMHO.

> Now whether that's a sane feature or not, I'm not sure.

would restricting the output of the credential helpers to username and
password make more sense in this context then?

obviously this change will required both the protocol (definition and
implementation) to change an changes on ALL helpers but I thought that was
the objective of that documentation suggestion in Jonathan's original email.

I agree though the discussion of if that is something we should do in a
maintenance release or not, is worth having; but I suspect it could be
implemented with reasonable enough backward compatibility to be considered.

Carlo

CC adding the author of cert:// too easy on that part of the discussion