Duplicat-word typos in code comments

On 4 Oct 2021, at 14:56, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

I noticed a duplicate-word typo in a comments recently, and cooked up
the following ripgrep command to find some more.

Pushed to master, thanks! I avoided the reflow of the comments though to make
it the minimal change.

--
Daniel Gustafsson https://vmware.com/

Daniel Gustafsson <daniel@yesql.se> writes:

On 4 Oct 2021, at 14:56, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

I noticed a duplicate-word typo in a comments recently, and cooked up
the following ripgrep command to find some more.

Pushed to master, thanks!

Thanks!

I avoided the reflow of the comments though to make it the minimal
change.

Fair enough. I wasn't sure myself whether to do it or not.

- ilmari

=?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org> writes:

Daniel Gustafsson <daniel@yesql.se> writes:

I avoided the reflow of the comments though to make it the minimal
change.

Fair enough. I wasn't sure myself whether to do it or not.

The next pgindent run will do it anyway (except in comment blocks
starting in column 1).

I used to think it was better to go ahead and manually reflow, if you
use an editor that makes that easy. That way there are fewer commits
touching any one line of code, which is good when trying to review
code history. However, now that we've got the ability to make "git
blame" ignore pgindent commits, maybe it's better to leave that sort
of mechanical cleanup to pgindent, so that the substantive patch is
easier to review.

(But I'm not sure how well the ignore-these-commits behavior actually
works for cases like this.)

regards, tom lane

On 4 Oct 2021, at 15:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:

I used to think it was better to go ahead and manually reflow, if you
use an editor that makes that easy. That way there are fewer commits
touching any one line of code, which is good when trying to review
code history. However, now that we've got the ability to make "git
blame" ignore pgindent commits, maybe it's better to leave that sort
of mechanical cleanup to pgindent, so that the substantive patch is
easier to review.

Yeah, that's precisely why I did it. Since we can skip over pgindent sweeps it
makes sense to try and minimize such changes to make code archaeology easier.
There are of course cases when the result will be such an eyesore that we'd
prefer to have it done sooner, but in cases like these where line just got one
word shorter it seemed an easy choice.

--
Daniel Gustafsson https://vmware.com/

Daniel Gustafsson <daniel@yesql.se> writes:

On 4 Oct 2021, at 15:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I used to think it was better to go ahead and manually reflow, if you
use an editor that makes that easy. That way there are fewer commits
touching any one line of code, which is good when trying to review
code history. However, now that we've got the ability to make "git
blame" ignore pgindent commits, maybe it's better to leave that sort
of mechanical cleanup to pgindent, so that the substantive patch is
easier to review.

Yeah, that's precisely why I did it. Since we can skip over pgindent sweeps it
makes sense to try and minimize such changes to make code archaeology easier.
There are of course cases when the result will be such an eyesore that we'd
prefer to have it done sooner, but in cases like these where line just got one
word shorter it seemed an easy choice.

Actually though, there's another consideration: if you leave
not-correctly-pgindented code laying around, it causes problems
for the next hacker who modifies that file and wishes to neaten
up their own work by pgindenting it. They can either tediously
reverse out part of the delta, or commit a patch that includes
entirely-unrelated cosmetic changes, neither of which is
pleasant.

regards, tom lane

On 4 Oct 2021, at 21:54, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Daniel Gustafsson <daniel@yesql.se> writes:

On 4 Oct 2021, at 15:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I used to think it was better to go ahead and manually reflow, if you
use an editor that makes that easy. That way there are fewer commits
touching any one line of code, which is good when trying to review
code history. However, now that we've got the ability to make "git
blame" ignore pgindent commits, maybe it's better to leave that sort
of mechanical cleanup to pgindent, so that the substantive patch is
easier to review.

Yeah, that's precisely why I did it. Since we can skip over pgindent sweeps it
makes sense to try and minimize such changes to make code archaeology easier.
There are of course cases when the result will be such an eyesore that we'd
prefer to have it done sooner, but in cases like these where line just got one
word shorter it seemed an easy choice.

Actually though, there's another consideration: if you leave
not-correctly-pgindented code laying around, it causes problems
for the next hacker who modifies that file and wishes to neaten
up their own work by pgindenting it. They can either tediously
reverse out part of the delta, or commit a patch that includes
entirely-unrelated cosmetic changes, neither of which is
pleasant.

Right, this is mainly targeting comments where changing a word on the first
line in an N line long comment can have the knock-on effect of changing N-1
lines just due to reflowing. This is analogous to wrapping existing code in a
new block, causing a re-indentation to happen, except that for comments it can
sometimes be Ok to leave (as in this particular case). At the end of the day,
it's all a case-by-case basis trade-off call.

--
Daniel Gustafsson https://vmware.com/