| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | solaimurugan vellaipandiyan <drsolaimurugan(dot)v(at)gmail(dot)com> |
| Cc: | Aleksander Alekseev <aleksander(at)tigerdata(dot)com>, Payal Singh <payals1(at)umbc(dot)edu>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Review - Patch for pg_bsd_indent: improve formatting of multiline comments |
| Date: | 2026-05-10 23:37:29 |
| Message-ID: | 1645542.1778456249@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
solaimurugan vellaipandiyan <drsolaimurugan(dot)v(at)gmail(dot)com> writes:
> While testing some real PostgreSQL source files, I noticed
> banner-style comments in contrib/seg/seg.c still receive formatting
> changes like:
> - This file contains routines ...
> + * This file contains routines ...
> This looks similar to the earlier discussion around separator-style
> comments and possible unnecessary diff churn. Since these header
> comments already appear visually structured, perhaps preserving them
> could help reduce additional formatting noise.
I think those changes are fine; the point of this patch is to
standardize our multiline comments, not to allow multiple styles
to persist. In fact, if anything I think the v7 patch isn't going
far enough. I wondered why it's making this exception:
+ # Only format comments that match the expected format,
+ # or at least that could have been the author's intent.
+ if ( ($lines[0] ne "/*" && $lines[-1] ne " */")
+ or ($lines[1] !~ m!^\s+\*!))
+ {
+ return $source;
+ }
I tried removing that, and soon found that it's the only thing
stopping the code from converting
/* foo bar */
to
/*
* foo bar
*/
which I don't think we want. So instead I replaced that bit with
an explicit test preventing reformatting a single-line comment.
After that it made a lot of changes that I thought were improvements,
but there were still some places where the output wasn't very uniform,
so I did some additional hacking to normalize the leading whitespace
and the number of '*' characters.
Attached are a proposed v8 of the patch, plus two diff files showing
the effects. v7-0001.diff.nocfbot is what the v7 patch does with
today's HEAD (it's the same as before). The v8 patch makes all those
changes and in addition makes the ones shown in v8-0001.diff.nocfbot.
I think those are pretty much all improvements, except that it kind
of messes up Martin Utesch's ASCII-art signatures in the geqo files.
That's because there are some lines starting with '*' and some with
'='. This is another place where I doubt it's worth the trouble to
try to make pgindent handle the case nicely; I propose just manually
adding leading '*'s to those comments before running pgindent.
regards, tom lane
| Attachment | Content-Type | Size |
|---|---|---|
| v8-0001-pgindent-improve-formatting-of-multiline-comments.patch | text/x-diff | 3.1 KB |
| v7-0001.diff.nocfbot | text/x-diff | 103.3 KB |
| v8-0001.diff.nocfbot | text/x-diff | 86.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alexander Korotkov | 2026-05-10 23:42:51 | Re: Dump statistic issue with index on expressions |
| Previous Message | Alexander Korotkov | 2026-05-10 22:22:58 | Re: Function scan FDW pushdown |