Re: [PATCH v2 09/34] sequencer (rebase -i): write an author-script file

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 19, 2016, 17:06 UTC
Message-ID: <alpine.DEB.2.20.1612191800530.54750@virtualbox>
In-Reply-To: <xmqqd1gtuivc.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Thu, 15 Dec 2016, Junio C Hamano wrote:

Show 25 quoted lines

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > +	strbuf_addstr(&buf, "GIT_AUTHOR_NAME='");
> > +	while (*message && *message != '\n' && *message != '\r')
> > +		if (skip_prefix(message, " <", &message))
> > +			break;
> > +		else if (*message != '\'')
> > +			strbuf_addch(&buf, *(message++));
> > +		else
> > +			strbuf_addf(&buf, "'\\\\%c'", *(message++));
> > +	strbuf_addstr(&buf, "'\nGIT_AUTHOR_EMAIL='");
> > +	while (*message && *message != '\n' && *message != '\r')
> > +		if (skip_prefix(message, "> ", &message))
> > +			break;
> > +		else if (*message != '\'')
> > +			strbuf_addch(&buf, *(message++));
> > +		else
> > +			strbuf_addf(&buf, "'\\\\%c'", *(message++));
> 
> Aren't these reading from an in-core commit object?  
> 
> If so, it should use split_ident_line() for consistency with other
> parts of the system to do this parsing.  We should also already have
> a helper for simple shell-quoting in quote.c and you would want to
> use that instead of open coding like this.

We keep coming back to the same argument. You want this quoting/dequoting to be turned into a full-fledged parser. And I keep pointing out that the code here does not *need* to parse but only construct an environment block.

Hopefully the next iteration that integrates Peff's suggestions will find more of your approval.

Ciao, Dscho