Re: [PATCH] Unknown cfg function

On 08/21/2013 09:34 AM, Tsantilas Christos wrote:

> - message << "Bungled " << f->filePath << " line " << f->lineNo <<
> + message << "Bungled (#1)" << f->filePath << " line " << f->lineNo <<

> - fatalf("Bungled %s line %d: %s",
> + fatalf("Bungled (#2) %s line %d: %s",

Please undo these changes.

> /**
> + * Preview the next token. The next NextToken() and strtokFile() call
> + * will return the same token.
> + * On parse error (eg invalid characters in token) will return an
> + * error message as token.
> */
> + static char *NextTokenPreview();

I suggest calling this method PeekAtToken() or TokenPeek() because many
standard APIs use "peek" to describe this "non-advancing read"
functionality. The Preview_ data member can stay as it is now -- no need
to rename that.

I appreciate you addressing my earlier concerns. I am unable to visually
verify all the low-level, parsing changes in the patch. I trust you have
tested this stuff the best you could.

Since Amos' already gave his +0, you may commit the changes to trunk if
you do not hear otherwise from anybody within 24 hours of your latest
email. Most of the callers changes are a significant improvement even if
there are low-level parsing bugs.

As I wrote earlier, the 'foo="bar and baz"' issue worries me, but I
think we can discuss that after your commit. The important part is for
Amos not to pull your changes into v3.4 until that discussion is over.

Thank you,

Alex.
Received on Wed Aug 28 2013 - 15:23:35 MDT