Re: [PATCH v6] credential-store: warn instead of fatal for bogus lines from store

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

On Wed, Apr 29, 2020 at 04:47:31PM -0700, Junio C Hamano wrote:
> > instead of doing a hard check for credentials, do a soft one and
> > warn the user so any invalid entries could be corrected.
> 
> instead -> instead

;)

> I do not think dropping "view or" is justifiable.  There is no need
> to invite the risk of opening with the intention to only view and
> then end up saving a modification.  In other words, do not encourage
> use of an *editor* in any way.
> 
> > +An unparseable or otherwise invalid line is ignored, and a warning
> > +message points out the problematic line number and file it appears in.
> 
> OK.  You didn't want to tell them they can remove the problematic
> line as a whole with their editor?

someone said "do not encourage use of an *editor* in any way" just a few
lines before ;)

> > +	test_config credential.helper store &&
> > +	git credential fill <<-\EOF >stdout 2>stderr &&
> > +	protocol=https
> > +	host=example.com
> > +	EOF
> > +	test_cmp expect-stdout stdout &&
> > +	test_i18ngrep "ignoring invalid credential" stderr &&
> > +	test_line_count = 3 stderr
> 
> The "ignoring invalid credential" message could be translated into
> two or more lines, but I think that is worrying too much about
> theoretical possibility, so checking line count would probably be
> sufficient.

yes, and specially considering that if I even ended up adding a -c
option to test_i18ngrep as I intended originally we will still the
same issue.

what is the preferred way to do something like this, or is it better
to add a check for each line formatting issue this is handling?

Carlo