public inbox for [email protected]  
help / color / mirror / Atom feed
From: Tom Lane <[email protected]>
To: Aleksander Alekseev <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Nathan Bossart <[email protected]>
Subject: Re: [PATCH] pg_bsd_indent: improve formatting of multiline comments
Date: Mon, 04 May 2026 12:40:19 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAJ7c6TPJVau3Ppih7+KubRwvG9CRpXq16L3X9bE02KuGgL95Nw@mail.gmail.com>
References: <[email protected]>
	<CAJ7c6TOCJG2p0Ob9Z7mXQRhkmRX_mxBay198Z173t4=PZr=yEQ@mail.gmail.com>
	<CAE7r3M+phC+kyJsUrJ6z97-UA0LXRrUBJ61hBk7WhRqdXF_itQ@mail.gmail.com>
	<CAJ7c6TOGdVBK5P-s59c77qxaw=ydPjpzp_s3VbpHeHn3Gc1iyA@mail.gmail.com>
	<aPqPK4ERy5-jj185@nathan>
	<CAJ7c6TMTzQsnKfM+_4i-4WO7=YnV4nO8ABWA6enSpdtg6DKhqQ@mail.gmail.com>
	<aP-H6kSsGOxaB21k@nathan>
	<[email protected]>
	<[email protected]>
	<aTs7A4lbcm0ThXaO@nathan>
	<aYZqyoNlah_E-Zua@nathan>
	<CAJ7c6TPJVau3Ppih7+KubRwvG9CRpXq16L3X9bE02KuGgL95Nw@mail.gmail.com>

Aleksander Alekseev <[email protected]> writes:
> Thanks, Nathan. Re-submitting v6 to make it visible by cfbot again and
> in order to simplify the work of reviewers a bit.

I tested the v6 patch by running it on current HEAD.  Pretty nearly all
the changes it makes are improvements, but it seems like there's still
some work to be done.  In particular, in src/port/inet_aton.c it
produces this diff:

--- a/src/port/inet_aton.c
+++ b/src/port/inet_aton.c
@@ -36,7 +37,8 @@
  * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
  * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.  */
+ * SUCH DAMAGE. 
+ */
 
 #include "c.h"

in which not all the trailing whitespace has been removed from the
first line, causing "git diff --check" to complain:

$ git diff --check
src/port/inet_aton.c:40: trailing whitespace.
+ * SUCH DAMAGE. 

In this case the problem is that the cleanup pattern only accounts for
one trailing space.  But a variant of this, which I think affects many
of the steps in the patch, is that there could be tab(s) there.  You
should fix the patterns to allow any number of spaces/tabs at the
spots where they currently expect just one space.  This might result
in finding cleanups they miss now.

Another amusing diff I noticed:

--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -5713,7 +5713,7 @@ check_parent_values_in_new_partitions(Relation parent,
  * 3. In case new partitions don't contain the DEFAULT partition and the
  *	  partitioned table does not have the DEFAULT partition, the following
  *	  should be true: the sum of the bounds of new partitions should be equal
- &	  to the bound of the split partition.
+ * &	  to the bound of the split partition.
  *
  * parent:			partitioned table
  * splitPartOid:	split partition Oid

Clearly, this is somebody's off-by-one-key typo, and the correct
fix is s/&/*/.  I suspect that fixing that manually is the most
expedient answer, rather than trying to make pg_bsd_indent smart
enough to DTRT.

One other nitpick is that the patch itself needs to be run through
pgperltidy (which has different opinions than your editor about
tabs vs spaces, apparently).

			regards, tom lane





view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] pg_bsd_indent: improve formatting of multiline comments
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox