pgsql: Preventive maintenance in advance of pgindent run.

Tom Lane wrote:

src/bin/pg_basebackup/pg_basebackup.c | 24 +++++++++---------
src/bin/pg_waldump/pg_waldump.c | 18 ++++++-------

There are some changes here that should be reverted; for instance:

-   printf(_("  -c, --checkpoint=fast|spread\n"
-            "                         set fast or spread checkpointing\n"));
+   printf(_("  -c, --checkpoint=fast|spread\n"));
+   printf(_("                         set fast or spread checkpointing\n"));

From the translator's point of view the patched version doesn't make
sense because they are two separate strings. In the original, it's a
single translatable string. Particularly in pg_waldump's -p, where a
phrase is now cut in the middle.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

There are some changes here that should be reverted; for instance:

-   printf(_("  -c, --checkpoint=fast|spread\n"
-            "                         set fast or spread checkpointing\n"));
+   printf(_("  -c, --checkpoint=fast|spread\n"));
+   printf(_("                         set fast or spread checkpointing\n"));

From the translator's point of view the patched version doesn't make
sense because they are two separate strings. In the original, it's a
single translatable string. Particularly in pg_waldump's -p, where a
phrase is now cut in the middle.

What I was concerned about was that pgindent will reindent the second
line so that it's impossible to tell whether the spacing is correct.
That might not matter to translators but it will be a problem for
source-level maintenance.

Maybe we should rethink the whole idea of breaking these entries across
lines, and just accept that the commentary doesn't line up with other
lines:

printf(_(" -c, --checkpoint=fast|spread set fast or spread checkpointing\n"));

Thoughts?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

... Particularly in pg_waldump's -p, where a
phrase is now cut in the middle.

BTW, I would say that the problem with -p is that somebody failed to
understand the difference between --help and a man page. That entry
should be

-p, --path=PATH directory in which to find log segment files

full stop. I'm not sure that the other multi-line entries in
pg_waldump's --help have an excuse to live, either.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

I wrote:

BTW, I would say that the problem with -p is that somebody failed to
understand the difference between --help and a man page.

Concretely, how about the attached? I don't think this looks any
worse than the current layout.

regards, tom lane

On 5/17/17 11:37, Tom Lane wrote:

I wrote:

BTW, I would say that the problem with -p is that somebody failed to
understand the difference between --help and a man page.

Concretely, how about the attached? I don't think this looks any
worse than the current layout.

The previous setup has been in place for years and has never been a
problem. The alternatives are all quite a bit worse.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 5/17/17 10:14, Tom Lane wrote:

What I was concerned about was that pgindent will reindent the second
line so that it's impossible to tell whether the spacing is correct.

pgindent moving string continuations to the left is a completely
terrible behavior anyway and we should look into changing that. Just
look at the mess it makes out of SELECT queries in pg_dump.c.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 5/17/17 11:37, Tom Lane wrote:

Concretely, how about the attached? I don't think this looks any
worse than the current layout.

The previous setup has been in place for years and has never been a
problem. The alternatives are all quite a bit worse.

No, the previous setup hasn't been "in place for years". These programs
were only NLS-ified last fall. Before that the code looked like, eg,

printf(" -z, --stats[=record] show statistics instead of records\n");
printf(" (optionally, show per-record statistics)\n");

so that there weren't string continuations for pgindent to fool with.

I'm not really convinced that having usage() print two- or three-line
switch descriptions is "quite a bit better" than what I suggested.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 5/17/17 10:14, Tom Lane wrote:

What I was concerned about was that pgindent will reindent the second
line so that it's impossible to tell whether the spacing is correct.

pgindent moving string continuations to the left is a completely
terrible behavior anyway and we should look into changing that. Just
look at the mess it makes out of SELECT queries in pg_dump.c.

I'd be on board with that, perhaps, but does anyone here have enough
insight into bsd_indent to fix that without breaking other stuff?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:

Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:

On 5/17/17 11:37, Tom Lane wrote:

Concretely, how about the attached? I don't think this looks any
worse than the current layout.

The previous setup has been in place for years and has never been a
problem. The alternatives are all quite a bit worse.

No, the previous setup hasn't been "in place for years". These programs
were only NLS-ified last fall.

We use the same technique in other places such as pg_dump's help() too.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Tom Lane wrote:

No, the previous setup hasn't been "in place for years". These programs
were only NLS-ified last fall.

We use the same technique in other places such as pg_dump's help() too.

Meh. Well, I reverted the changes in question while we discuss it.

Changing the pgindent rule for such cases sounds kind of promising,
but will anyone pursue it?

(In any case, we should probably go ahead with the scheduled pgindent
run, and then we can consider a new run with new rules later; that
will ease seeing exactly what changes a new rule would make.)

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:

Changing the pgindent rule for such cases sounds kind of promising,
but will anyone pursue it?

We have someone who has studied the BSD indent code and even sent us
patches to fix quite a few bugs, but we've largely ignored his efforts
so far. I propose we take that indent as part of our repo, and patch it
to our preferences.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

We have someone who has studied the BSD indent code and even sent us
patches to fix quite a few bugs, but we've largely ignored his efforts
so far. I propose we take that indent as part of our repo, and patch it
to our preferences.

Messing with pgindent didn't seem all that high-priority while we were
in development mode, and it would also have been pretty painful to have
a pgindent that wanted to revisit settled decisions when you just wanted
it to fix new code. Maybe now (ie, over the next few weeks) is a good
time to pursue it, before we start a new round of code development.

Not sure about actually incorporating it into our repo. Doing so would
make it easier for people to use, for sure, and the license seems to be
regular 3-clause BSD, so that angle is OK. But do we want to be carrying
around another 150K of source code?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, May 17, 2017 at 01:06:26PM -0400, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Tom Lane wrote:

No, the previous setup hasn't been "in place for years". These programs
were only NLS-ified last fall.

We use the same technique in other places such as pg_dump's help() too.

Meh. Well, I reverted the changes in question while we discuss it.

Are we ready for a pgindent run now?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017-05-17 19:16, Alvaro Herrera wrote:

Tom Lane wrote:

Changing the pgindent rule for such cases sounds kind of promising,
but will anyone pursue it?

We have someone who has studied the BSD indent code and even sent us
patches to fix quite a few bugs, but we've largely ignored his efforts
so far. I propose we take that indent as part of our repo, and patch it
to our preferences.

That, I assume, would be me. Coincidentally, I'm about to push my fixes
upstream (FreeBSD). Before that happens, my changes can be obtained from
https://github.com/pstef/freebsd_indent and tested, if anyone wishes.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Bruce Momjian <bruce@momjian.us> writes:

Are we ready for a pgindent run now?

Yes, might as well do it. Some of these discussions might lead to
a re-run with a newer version of pgindent, but it would be good to
have a clean tree to start from.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

We have someone who has studied the BSD indent code and even sent us
patches to fix quite a few bugs, but we've largely ignored his efforts
so far. I propose we take that indent as part of our repo, and patch it
to our preferences.

Messing with pgindent didn't seem all that high-priority while we were
in development mode, and it would also have been pretty painful to have
a pgindent that wanted to revisit settled decisions when you just wanted
it to fix new code. Maybe now (ie, over the next few weeks) is a good
time to pursue it, before we start a new round of code development.

Sounds reasonable.

Not sure about actually incorporating it into our repo. Doing so would
make it easier for people to use, for sure, and the license seems to be
regular 3-clause BSD, so that angle is OK. But do we want to be carrying
around another 150K of source code?

The alternatives are

1. rely on the dead code we've been using so far (the old BSD indent
patched with our Pg-specific tweaks), or

2. rely on someone else's upstream code -- in this case, FreeBSD's as
patched by Piotr.

Now that Piotr's is about to find a home, perhaps it's okay for us to
rely on that one. I just didn't like the idea of running something from
Piotr's personal repo.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom, all,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Bruce Momjian <bruce@momjian.us> writes:

Are we ready for a pgindent run now?

Yes, might as well do it. Some of these discussions might lead to
a re-run with a newer version of pgindent, but it would be good to
have a clean tree to start from.

+1.

Thanks!

Stephen

Piotr Stefaniak <postgres@piotr-stefaniak.me> writes:

On 2017-05-17 19:16, Alvaro Herrera wrote:

We have someone who has studied the BSD indent code and even sent us
patches to fix quite a few bugs, but we've largely ignored his efforts
so far. I propose we take that indent as part of our repo, and patch it
to our preferences.

That, I assume, would be me. Coincidentally, I'm about to push my fixes
upstream (FreeBSD). Before that happens, my changes can be obtained from
https://github.com/pstef/freebsd_indent and tested, if anyone wishes.

Yes, I was just reviewing those threads. IIUC, the current proposal is
to adopt FreeBSD's version of indent as being less buggy and better
maintained than NetBSD's, and then patch it as necessary.

We'd put off the decision what to do exactly until a more suitable time,
but I think that time is now. What I think we ought to do is go ahead
and indent the tree with current pgindent, and then we have a basis for
experimenting with replacement versions and seeing what they'll do
differently. If we can make a decision and adopt any changes within
the next few weeks, I think that that timing will be about the least pain
we can hope for.

If we do anything with as much impact as changing the indentation rule
for string continuations, I will certainly vote for running the new
pgindent over the back branches too. I still bear the scars of dealing
with the comment-reflowing changes that Bruce put in circa 8.1 --- that
broke just about every back-patching effort for the next five years.
I don't want to go through that again.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017-05-17 13:35:22 -0400, Tom Lane wrote:

Not sure about actually incorporating it into our repo. Doing so would
make it easier for people to use, for sure, and the license seems to be
regular 3-clause BSD, so that angle is OK. But do we want to be carrying
around another 150K of source code?

150k? Isn't it like 3-4k?

- Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers