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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

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

> Johannes Schindelin <johannes.schindelin@xxxxxx> 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]