The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: not tested
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
Note: Feature implementation was tested and passes. Code style/compliance verified through review. installcheck-world
anddocumentation testing were not performed (neither required for this patch type per PostgreSQL conventions for
tool-onlychanges).
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.
