Review - Patch for pg_bsd_indent: improve formatting of multiline comments - Mailing list pgsql-hackers

From Payal Singh
Subject Review - Patch for pg_bsd_indent: improve formatting of multiline comments
Date
Msg-id EB0141C5-ACC2-4F0B-85EA-0E3AFBCE322F@umbc.edu
Whole thread Raw
Responses Re: Review - Patch for pg_bsd_indent: improve formatting of multiline comments
List pgsql-hackers

This is a review of the pgindent multiline comments patch:

https://www.postgresql.org/message-id/attachment/189778/v6-0001-pgindent-improve-formatting-of-multiline-comments.patch


Contents & Purpose

==================

This patch adds a `postprocess_multiline_comment()` function to pgindent

that reformats multiline comments to put `/*` on its own line:


  Before: /* This is line 1

   * This is line 2

   */


  After:  /*

   * This is line 1

   * This is line 2

   */


The patch correctly excludes Doxygen (`/**`), compat flag (`/*-`), and

separator patterns (`/* ===`, `/* ---`).


Initial Run

===========

The patch applies cleanly to HEAD. I tested with various comment styles

and all cases passed:


- Basic multiline: reformatted correctly

- Doxygen/compat: left unchanged (correct)

- Separator lines: preserved (correct)

- Single-line comments: left unchanged (correct)

- Already-correct format: left unchanged (correct)


The validation logic at line 303 correctly requires `/*` on its own

line and ` */` on its own line before processing. Comments that don't

match this pattern are returned unchanged.


Performance

===========

Tested on a 74KB C file: 0.038 seconds. Negligible impact; the regex

only runs on comments.


Nitpicking & Conclusion

=======================

Minor: The regex notation in the report (line 85) shows `!` as part of

the pattern when it's actually the delimiter. The code itself is correct.


The validation description could be clearer that it returns early (skips

processing) when conditions aren't met, rather than "requires" them.


Otherwise this is a clean, minimal patch (36 lines) that fits well with

existing pgindent patterns. Single file change, no build system

modifications, developer tool only.

pgsql-hackers by date:

Previous
From: Yura Sokolov
Date:
Subject: Re: BM_IO_ERROR flag is lost in TerminateBufferIO due to order of operations in UnlockBufHdrExt
Next
From: Srinath Reddy Sadipiralla
Date:
Subject: Re: Adding REPACK [concurrently]