pgindent
I think it's about time for us to run pgindent. I did a trial run
today of pgindent today and came up with the attached patch for
typedefs.list, which I'd like to commit more or less immediately,
barring objections. It mostly just adds new typedefs that have
appeared over the last year, but it also realphabetizes the file -
some things that were added incrementally seem to have ended up in
what is, at least according to what sort likes to do on my machine,
the wrong place in the file.
With this applied, I get a fairly clean pgindent run. There are some
problems with comments getting mangled, and in a couple of cases
function definitions getting mangled, that need more investigation.
I'll try to find time to look into that soon and follow up, unless
somebody else beats me to it. As far as possible, I think it's
desirable to clean up those things before rather than after running
pgindent, because unmangling ASCII art that pgindent has stepped on is
a thankless chore.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
typedefs-cleanup.patchapplication/x-download; name=typedefs-cleanup.patchDownload+101-8
On Wed, Apr 27, 2016 at 10:51:55AM -0400, Robert Haas wrote:
I think it's about time for us to run pgindent. I did a trial run
today of pgindent today and came up with the attached patch for
typedefs.list, which I'd like to commit more or less immediately,
barring objections. It mostly just adds new typedefs that have
appeared over the last year, but it also realphabetizes the file -
some things that were added incrementally seem to have ended up in
what is, at least according to what sort likes to do on my machine,
the wrong place in the file.With this applied, I get a fairly clean pgindent run. There are some
problems with comments getting mangled, and in a couple of cases
function definitions getting mangled, that need more investigation.
I'll try to find time to look into that soon and follow up, unless
somebody else beats me to it. As far as possible, I think it's
desirable to clean up those things before rather than after running
pgindent, because unmangling ASCII art that pgindent has stepped on is
a thankless chore.
Agreed. Sounds like a good plan --- a better plan than I have used in
the past.
--
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 2016-04-27 10:51:55 -0400, Robert Haas wrote:
I think it's about time for us to run pgindent. Sounds reasonable.
I did a trial run
today of pgindent today and came up with the attached patch for
typedefs.list, which I'd like to commit more or less immediately,
barring objections.
Yes, that makes sense. That way other can easily look at "their" code,
to see whether it can be made more pgindent resistant ;)
It mostly just adds new typedefs that have
appeared over the last year, but it also realphabetizes the file -
some things that were added incrementally seem to have ended up in
what is, at least according to what sort likes to do on my machine,
the wrong place in the file.
Is it just me, or is the sort order in that file a bit confusing? The
whole thing about upper and lower case being separated seems to make it
much harder than necessary to manually insert something in the right
place..
- Andres
--
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, Apr 27, 2016 at 11:45 AM, Andres Freund <andres@anarazel.de> wrote:
Yes, that makes sense. That way other can easily look at "their" code,
to see whether it can be made more pgindent resistant ;)
Right.
It mostly just adds new typedefs that have
appeared over the last year, but it also realphabetizes the file -
some things that were added incrementally seem to have ended up in
what is, at least according to what sort likes to do on my machine,
the wrong place in the file.Is it just me, or is the sort order in that file a bit confusing? The
whole thing about upper and lower case being separated seems to make it
much harder than necessary to manually insert something in the right
place..
Except for recently-manually-added entries, it seems to match what
sort wants to do on my system exactly. Which seems good.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
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, Apr 27, 2016 at 10:57 AM, Bruce Momjian <bruce@momjian.us> wrote:
Agreed. Sounds like a good plan --- a better plan than I have used in
the past.
Thinking about this a bit more, what I am inclined to do is:
1. Run pgindent.
2. Read the diff and revert any changes that look icky.
3. Commit the rest.
4. Run pgindent again and post the diff. Discuss how to fix the
ickiness contained therein, then proceed accordingly.
How does that sound?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
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, Apr 27, 2016 at 11:51:42AM -0400, Robert Haas wrote:
It mostly just adds new typedefs that have
appeared over the last year, but it also realphabetizes the file -
some things that were added incrementally seem to have ended up in
what is, at least according to what sort likes to do on my machine,
the wrong place in the file.Is it just me, or is the sort order in that file a bit confusing? The
whole thing about upper and lower case being separated seems to make it
much harder than necessary to manually insert something in the right
place..Except for recently-manually-added entries, it seems to match what
sort wants to do on my system exactly. Which seems good.
Well, sort ordering is all related to your collation setting:
$ echo "a
A" | LC_COLLATE="" sort
a
A
$ echo "a
A" | LC_COLLATE="en_US" sort
A
a
--
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 Wed, Apr 27, 2016 at 11:57 AM, Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Apr 27, 2016 at 11:51:42AM -0400, Robert Haas wrote:
It mostly just adds new typedefs that have
appeared over the last year, but it also realphabetizes the file -
some things that were added incrementally seem to have ended up in
what is, at least according to what sort likes to do on my machine,
the wrong place in the file.Is it just me, or is the sort order in that file a bit confusing? The
whole thing about upper and lower case being separated seems to make it
much harder than necessary to manually insert something in the right
place..Except for recently-manually-added entries, it seems to match what
sort wants to do on my system exactly. Which seems good.Well, sort ordering is all related to your collation setting:
$ echo "a
A" | LC_COLLATE="" sort
a
A$ echo "a
A" | LC_COLLATE="en_US" sort
A
a
Sure. I guess we could resort that file with LC_COLLATE=C. But my
point was mostly just that the ordering is hardly haphazard.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
I think it's about time for us to run pgindent. I did a trial run
today of pgindent today and came up with the attached patch for
typedefs.list, which I'd like to commit more or less immediately,
barring objections.
Um, we normally take the buildfarm's list of typedefs, not anything
manually created.
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, Apr 27, 2016 at 2:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
I think it's about time for us to run pgindent. I did a trial run
today of pgindent today and came up with the attached patch for
typedefs.list, which I'd like to commit more or less immediately,
barring objections.Um, we normally take the buildfarm's list of typedefs, not anything
manually created.
Well, we can still do that, but I don't see much advantage in it. It
just churns the file to the extent that manual review of the changes
is impossible, and then when pgindent does the wrong thing it only
gets reported after the fact. How is that better than making sure
that the contents of the file are such as to actually produce good
output from pgindent?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
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, Apr 27, 2016 at 02:54:35PM -0400, Robert Haas wrote:
On Wed, Apr 27, 2016 at 2:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
I think it's about time for us to run pgindent. I did a trial run
today of pgindent today and came up with the attached patch for
typedefs.list, which I'd like to commit more or less immediately,
barring objections.Um, we normally take the buildfarm's list of typedefs, not anything
manually created.Well, we can still do that, but I don't see much advantage in it. It
just churns the file to the extent that manual review of the changes
is impossible, and then when pgindent does the wrong thing it only
gets reported after the fact. How is that better than making sure
that the contents of the file are such as to actually produce good
output from pgindent?
Using the buildfarm typedefs assures they are always generated in a
consistent way.
--
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 Wed, Apr 27, 2016 at 05:01:14PM -0400, Bruce Momjian wrote:
On Wed, Apr 27, 2016 at 02:54:35PM -0400, Robert Haas wrote:
On Wed, Apr 27, 2016 at 2:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
I think it's about time for us to run pgindent. I did a trial run
today of pgindent today and came up with the attached patch for
typedefs.list, which I'd like to commit more or less immediately,
barring objections.Um, we normally take the buildfarm's list of typedefs, not anything
manually created.Well, we can still do that, but I don't see much advantage in it. It
just churns the file to the extent that manual review of the changes
is impossible, and then when pgindent does the wrong thing it only
gets reported after the fact. How is that better than making sure
that the contents of the file are such as to actually produce good
output from pgindent?Using the buildfarm typedefs assures they are always generated in a
consistent way.
Oh, and as I remember the buildfarm merges several platforms, including
Windows, to make that list, so I suggest you use that one.
--
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
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Apr 27, 2016 at 2:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Um, we normally take the buildfarm's list of typedefs, not anything
manually created.
Well, we can still do that, but I don't see much advantage in it. It
just churns the file to the extent that manual review of the changes
is impossible, and then when pgindent does the wrong thing it only
gets reported after the fact. How is that better than making sure
that the contents of the file are such as to actually produce good
output from pgindent?
On what grounds do you claim the buildfarm result is unstable?
I've been using that for a long time and it works fine. Moreover,
ignoring that data is a bad idea because it reflects platform-specific
variations in the set of typedefs that are known. If you build a
typedefs list based only on what works on your machine, it likely
won't work for other people.
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, Apr 27, 2016 at 6:10 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Apr 27, 2016 at 2:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Um, we normally take the buildfarm's list of typedefs, not anything
manually created.Well, we can still do that, but I don't see much advantage in it. It
just churns the file to the extent that manual review of the changes
is impossible, and then when pgindent does the wrong thing it only
gets reported after the fact. How is that better than making sure
that the contents of the file are such as to actually produce good
output from pgindent?On what grounds do you claim the buildfarm result is unstable?
I've been using that for a long time and it works fine. Moreover,
ignoring that data is a bad idea because it reflects platform-specific
variations in the set of typedefs that are known. If you build a
typedefs list based only on what works on your machine, it likely
won't work for other people.
/me shrugs
Well, let's get the list, then, and compare it to what's in the file
now. How do we do that exactly?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
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, Apr 27, 2016 at 11:38:57PM -0400, Robert Haas wrote:
On what grounds do you claim the buildfarm result is unstable?
I've been using that for a long time and it works fine. Moreover,
ignoring that data is a bad idea because it reflects platform-specific
variations in the set of typedefs that are known. If you build a
typedefs list based only on what works on your machine, it likely
won't work for other people./me shrugs
Well, let's get the list, then, and compare it to what's in the file
now. How do we do that exactly?
The URL is in the file src/tools/pgindent/README:
5) Download the typedef file from the buildfarm:
wget -O src/tools/pgindent/typedefs.list http://buildfarm.postgresql.org/cgi-bin/typedefs.pl
(see http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list for a full list of typedefs,
also http://adpgtech.blogspot.com/2015/05/running-pgindent-on-non-core-code-or.html)
--
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 Thu, Apr 28, 2016 at 7:39 AM, Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Apr 27, 2016 at 11:38:57PM -0400, Robert Haas wrote:
On what grounds do you claim the buildfarm result is unstable?
I've been using that for a long time and it works fine. Moreover,
ignoring that data is a bad idea because it reflects platform-specific
variations in the set of typedefs that are known. If you build a
typedefs list based only on what works on your machine, it likely
won't work for other people./me shrugs
Well, let's get the list, then, and compare it to what's in the file
now. How do we do that exactly?The URL is in the file src/tools/pgindent/README:
5) Download the typedef file from the buildfarm:
wget -O src/tools/pgindent/typedefs.list http://buildfarm.postgresql.org/cgi-bin/typedefs.pl
(see http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list for a full list of typedefs,
also http://adpgtech.blogspot.com/2015/05/running-pgindent-on-non-core-code-or.html)
I compared the result of running pgindent with the typedefs.list file
as updated by me manually with the result of running pgindent using
the buildfarm list and ... the buildfarm list is better. Shows what I
know. Should we go ahead and commit the current version of that file
as src/tools/pgindent/typedefs.list, then?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
I compared the result of running pgindent with the typedefs.list file
as updated by me manually with the result of running pgindent using
the buildfarm list and ... the buildfarm list is better. Shows what I
know. Should we go ahead and commit the current version of that file
as src/tools/pgindent/typedefs.list, then?
Yes, if it's what you plan to use for pgindent'ing.
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