The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation: not tested
NEEDS CATVERSION BUMP
I editorialized the docs and some of the comments. In particular, I documented behavior of not truncating, and recommended casting to name[] if user cares about that. Added a unit test to verify that works. BTW, I saw mention in the thread about not truncated spaces, but the function *does* truncate them, unless they're inside quotes, where they're legitimate.
ok
Also added test for invalid characters.
I think "strict" would be more in line with other uses in code. There are currently no other occurrences of 'strictmode' in the code. There are loads of references to 'strict', but I didn't go through all of them to see if any were used as externally visible function parameter names.
I am sorry, I don't understand to this point. You unlike the name of parameter "strictmode" ? Have you any proposal? Maybe "restrictive" ?
qualname_str is used in exactly 1 place. Either it should be gotten rid of, or all the uses of text_to_cstring(qualname) should be changed to qualname_str.
fixed, qualname_str is used everywhere
I think the code would have been clearer if instead of the big if (*nextp == '\"') it did the same "inquote" looping that is done elsewhere, but I don't have a strong opinion on it.
The almost all code +/- is related to human readable error messages. We can move some code to separate static functions - read_quoted_ident, read_unquoted_ident, but there will be some magic about parameters, and the code will not be much better, than it is now.
The new status of this patch is: Waiting on Author