On Sat, Nov 12, 2022 at 09:35:59PM +0100, Pavel Stehule wrote:
Thanks for the updated patch. Apart from the function comment it looks good to
me.
Justin, did you have any other comment on the patch?
> > I don't fully understand the part about subpatterns, but is that necessary
> > to
> > describe it? Simply saying that any valid and possibly-quoted identifier
> > can
> > be parsed should make it clear that identifiers containing \n characters
> > should
> > work too. Maybe also just mention that whitespaces are removed and special
> > care is taken to output routines in exactly the same way calling code will
> > expect it (that is comma-and-single-space type delimiter).
> >
>
> In this case I hit the limits of my English language skills.
>
> I rewrote this comment, but it needs more care. Please, can you look at it?
I'm also not a native English speaker so I'm far for writing perfect comments
myself :)
Maybe something like
/*
* read_pattern - reads on object pattern from input
*
* This function will parse any valid identifier (quoted or not, qualified or
* not), which can also includes the full signature for routines.
* Note that this function takes special care to sanitize the detected
* identifier (removing extraneous whitespaces or other unnecessary
* characters). This is necessary as most backup/restore filtering functions
* only recognize identifiers if they are written exactly way as they are
* regenerated.
* Returns a pointer to next character after the found identifier, or NULL on
* error.
*/
