On 27.03.22 22:19, Tom Lane wrote:
Here's a rebase up to today's HEAD.  I've fixed the merge problems,
but there may be some stray new error calls that could be converted
to use pg_fatal() and haven't been.  I don't want to do a full
fresh scan of the code until we're about ready to commit this.

This looks like a good improvement to me.

I think I would want the program name/location also in front of the detail and hint lines. I need to think about this a bit more. This shouldn't hold up this patch; it would be a quick localized change. (I'm also thinking about providing a separate color code for the secondary messages. Again, this could be a quick follow-up patch.)

The one change I didn't like was

- pg_log_error("The program \"%s\" is needed by %s but was not found in the\n" - "same directory as \"%s\".\n"
-                                                "Check your installation.",
+ pg_log_error("the program \"%s\" is needed by %s but was not found in the same directory as \"%s\"", "postgres", progname, full_path);

This appears to prioritize the guideline "don't punctuate error message as full sentence" over what should be the actual guideline "don't make the error message a full sentence".

There are other occurrences of a similar message that were not changed in the same way by the patch. Maybe we should leave this one alone in this patch and consider rewriting the message instead.


Reply via email to