Message-ID: <6acbce27-3472-fc58-538b-ba6770ed0bfa@enterprisedb.com>
Date: Tue, 29 Mar 2022 16:24:18 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.7.0
Subject: Re: Frontend error logging style
Content-Language: en-US
To: Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>
Cc: Robert Haas <robertmhaas@gmail.com>, Michael Paquier
 <michael@paquier.xyz>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <YZMfMI3kTDw/RU0k@paquier.xyz>
 <CA+TgmoZy84orCfUNipEp+QzWwRuR2yrjsdcJN9ROHzPkXDRbxQ@mail.gmail.com>
 <a06f311b-18cd-ee56-ad4c-03ed0f93713f@enterprisedb.com>
 <CA+TgmoYaA1wtU91yNg1_C6oS8azAfp5dQ1YrfuNAm2=Yefrn1A@mail.gmail.com>
 <913687.1645572199@sss.pgh.pa.us>
 <6fdb3ab3-f093-27b9-dfdc-c391e69163fc@enterprisedb.com>
 <1346360.1645715441@sss.pgh.pa.us> <1895619.1645809325@sss.pgh.pa.us>
 <20220322015135.k4tcd3tsjq5xydqa@alap3.anarazel.de>
 <2446238.1647914437@sss.pgh.pa.us>
 <20220322021348.pvtvgrseyfohkxob@alap3.anarazel.de>
 <471299.1648412393@sss.pgh.pa.us>
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
In-Reply-To: <471299.1648412393@sss.pgh.pa.us>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/6acbce27-3472-fc58-538b-ba6770ed0bfa%40enterprisedb.com>
Precedence: bulk

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.