pgindent

On 04/08/2011 06:05 PM, Robert Haas wrote:

So, we talked about running pgindent a few weeks ago, but reading over
the thread, I guess we're still waiting for Andrew to update the list
of typedefs?

It would be really nice to get this done. Andrew, is there any chance
you can knock that out?

Yeah. There are three animals reporting (running Linux, FreeBSD and
MinGW builds). My Cygwin animal should report within the hour, and I'm
working on getting an MSVC build into the mix for the first time ever.
That should be done within the next 24 hours.

As soon as it is I'll commit the consolidated list.

cheers

andrew

On Fri, Apr 8, 2011 at 8:05 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 04/08/2011 06:05 PM, Robert Haas wrote:

So, we talked about running pgindent a few weeks ago, but reading over
the thread, I guess we're still waiting for Andrew to update the list
of typedefs?

It would be really nice to get this done.  Andrew, is there any chance
you can knock that out?

Yeah. There are three animals reporting (running Linux, FreeBSD and MinGW
builds). My Cygwin animal should report within the hour, and I'm working on
getting an MSVC build into the mix for the first time ever. That should be
done within the next 24 hours.

As soon as it is I'll commit the consolidated list.

Thanks!

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 04/08/2011 10:12 PM, Robert Haas wrote:

On Fri, Apr 8, 2011 at 8:05 PM, Andrew Dunstan<andrew@dunslane.net> wrote:

On 04/08/2011 06:05 PM, Robert Haas wrote:

So, we talked about running pgindent a few weeks ago, but reading over
the thread, I guess we're still waiting for Andrew to update the list
of typedefs?

It would be really nice to get this done. Andrew, is there any chance
you can knock that out?

Yeah. There are three animals reporting (running Linux, FreeBSD and MinGW
builds). My Cygwin animal should report within the hour, and I'm working on
getting an MSVC build into the mix for the first time ever. That should be
done within the next 24 hours.

As soon as it is I'll commit the consolidated list.

Thanks!

We've got more work to do before that works, so I have committed what we
have. Some symbols have disappeared, some because of code changes and
some probably because Cygwin has changed the way it does objdump. This
is probably harmless, but whoever does the pgindent run needs to look at
the results carefully before committing them (as always).

cheers

andrew

On Fri, Apr 8, 2011 at 11:21 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

We've got more work to do before that works, so I have committed what we
have. Some symbols have disappeared, some because of code changes and some
probably because Cygwin has changed the way it does objdump. This is
probably harmless, but whoever does the pgindent run needs to look at the
results carefully before committing them (as always).

Well, that's normally Bruce. Bruce?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas wrote:

On Fri, Apr 8, 2011 at 11:21 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

We've got more work to do before that works, so I have committed what we
have. Some symbols have disappeared, some because of code changes and some
probably because Cygwin has changed the way it does objdump. This is
probably harmless, but whoever does the pgindent run needs to look at the
results carefully before committing them (as always).

Well, that's normally Bruce. Bruce?

I can run it tonight, in 15 hours.

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

+ It's impossible for everything to be true. +

Bruce Momjian wrote:

Robert Haas wrote:

On Fri, Apr 8, 2011 at 11:21 PM, Andrew Dunstan <andrew@dunslane.net> wrote:

We've got more work to do before that works, so I have committed what we
have. Some symbols have disappeared, some because of code changes and some
probably because Cygwin has changed the way it does objdump. This is
probably harmless, but whoever does the pgindent run needs to look at the
results carefully before committing them (as always).

Well, that's normally Bruce. Bruce?

I can run it tonight, in 15 hours.

27 hours later, done. I ran all the tests outlined in the pgindent
README.

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

+ It's impossible for everything to be true. +

On Sun, Apr 10, 2011 at 4:42 PM, Bruce Momjian <bruce@momjian.us> wrote:

27 hours later, done.   I ran all the tests outlined in the pgindent
README.

What's with things like:

-void _PG_init(void);
+void       _PG_init(void);

-   Datum diff = DirectFunctionCall2(date_mi,
+   Datum       diff = DirectFunctionCall2(date_mi,

    const TimeADT *aa = (const TimeADT *) a;
    const TimeADT *bb = (const TimeADT *) b;
-   Interval      *i;
+   Interval   *i;

Note that in the last one someone carefully made the variable names
line up and pgindent is changing the spacing to an arbitrary amount.

--
greg

And this doesn't seem helpful:

    /* Format options */
    /* oids option is not supported */
-   { "format",         ForeignTableRelationId },
-   { "header",         ForeignTableRelationId },
-   { "delimiter",      ForeignTableRelationId },
-   { "quote",          ForeignTableRelationId },
-   { "escape",         ForeignTableRelationId },
-   { "null",           ForeignTableRelationId },
-   { "encoding",       ForeignTableRelationId },
+   {"format", ForeignTableRelationId},
+   {"header", ForeignTableRelationId},
+   {"delimiter", ForeignTableRelationId},
+   {"quote", ForeignTableRelationId},
+   {"escape", ForeignTableRelationId},
+   {"null", ForeignTableRelationId},
+   {"encoding", ForeignTableRelationId},

--
greg

On Sun, Apr 10, 2011 at 11:55 AM, Greg Stark <gsstark@mit.edu> wrote:

On Sun, Apr 10, 2011 at 4:42 PM, Bruce Momjian <bruce@momjian.us> wrote:

27 hours later, done.   I ran all the tests outlined in the pgindent
README.

What's with things like:

-void _PG_init(void);
+void       _PG_init(void);

-   Datum diff = DirectFunctionCall2(date_mi,
+   Datum       diff = DirectFunctionCall2(date_mi,

   const TimeADT *aa = (const TimeADT *) a;
   const TimeADT *bb = (const TimeADT *) b;
-   Interval      *i;
+   Interval   *i;

Note that in the last one someone carefully made the variable names
line up and pgindent is changing the spacing to an arbitrary amount.

Well, it's the same arbitrary amount that we use throughout our code,
presumably. I am not sure whether pgident is the best tool for the
job, but at least it makes the code relatively consistent throughout,
which is mostly a good thing.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

On Sun, Apr 10, 2011 at 11:55 AM, Greg Stark <gsstark@mit.edu> wrote:

Note that in the last one someone carefully made the variable names
line up and pgindent is changing the spacing to an arbitrary amount.

Well, it's the same arbitrary amount that we use throughout our code,
presumably. I am not sure whether pgident is the best tool for the
job, but at least it makes the code relatively consistent throughout,
which is mostly a good thing.

Yes. pgindent has never been about preserving somebody else's idea
of what's appropriate formatting. This is sometimes bad but on the
whole it seems to be a win.

What I was a bit surprised by is the volume of changes in wparser_def.c
--- so far as I can see, that file hardly changed since 9.0, so why did
pgindent suddenly whack it around so much?  The other files that changed
a lot are mostly new code so widespread changes are unsurprising.

regards, tom lane

Robert Haas wrote:

On Sun, Apr 10, 2011 at 11:55 AM, Greg Stark <gsstark@mit.edu> wrote:

On Sun, Apr 10, 2011 at 4:42 PM, Bruce Momjian <bruce@momjian.us> wrote:

27 hours later, done. ? I ran all the tests outlined in the pgindent
README.

What's with things like:

-void _PG_init(void);
+void ? ? ? _PG_init(void);

- ? Datum diff = DirectFunctionCall2(date_mi,
+ ? Datum ? ? ? diff = DirectFunctionCall2(date_mi,

? ?const TimeADT *aa = (const TimeADT *) a;
? ?const TimeADT *bb = (const TimeADT *) b;
- ? Interval ? ? ?*i;
+ ? Interval ? *i;

Note that in the last one someone carefully made the variable names
line up and pgindent is changing the spacing to an arbitrary amount.

Well, it's the same arbitrary amount that we use throughout our code,
presumably. I am not sure whether pgident is the best tool for the
job, but at least it makes the code relatively consistent throughout,
which is mostly a good thing.

Yes, there are going to be negative aspects of pgindent --- the big
question is whether it is a net gain or loss.

I have always felt it is radical that we do this --- good, but radical.

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

+ It's impossible for everything to be true. +

On 04/10/2011 12:11 PM, Tom Lane wrote:

Robert Haas<robertmhaas@gmail.com> writes:

On Sun, Apr 10, 2011 at 11:55 AM, Greg Stark<gsstark@mit.edu> wrote:

Note that in the last one someone carefully made the variable names
line up and pgindent is changing the spacing to an arbitrary amount.

Well, it's the same arbitrary amount that we use throughout our code,
presumably. I am not sure whether pgident is the best tool for the
job, but at least it makes the code relatively consistent throughout,
which is mostly a good thing.

Yes. pgindent has never been about preserving somebody else's idea
of what's appropriate formatting. This is sometimes bad but on the
whole it seems to be a win.

What I was a bit surprised by is the volume of changes in wparser_def.c
--- so far as I can see, that file hardly changed since 9.0, so why did
pgindent suddenly whack it around so much?  The other files that changed
a lot are mostly new code so widespread changes are unsurprising.

I had a dim and possibly erroneous recollection that was one of the
files we excluded from pgindent runs. If you look at the history it
hasn't been touched by pgindent since the 8.3 run. But the changes to it
all look fairly kosher at first glance.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

On 04/10/2011 12:11 PM, Tom Lane wrote:

What I was a bit surprised by is the volume of changes in wparser_def.c
--- so far as I can see, that file hardly changed since 9.0, so why did
pgindent suddenly whack it around so much?  The other files that changed
a lot are mostly new code so widespread changes are unsurprising.

I had a dim and possibly erroneous recollection that was one of the
files we excluded from pgindent runs. If you look at the history it
hasn't been touched by pgindent since the 8.3 run. But the changes to it
all look fairly kosher at first glance.

Oh, I bet you're right -- see commit
97116ca4170b974d734cea364789c389b30e6ce1. That removed the exclusion
but I don't see any evidence that Bruce actually pgindent'd it then.

regards, tom lane

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

On 04/10/2011 12:11 PM, Tom Lane wrote:

What I was a bit surprised by is the volume of changes in wparser_def.c
--- so far as I can see, that file hardly changed since 9.0, so why did
pgindent suddenly whack it around so much?  The other files that changed
a lot are mostly new code so widespread changes are unsurprising.

I had a dim and possibly erroneous recollection that was one of the
files we excluded from pgindent runs. If you look at the history it
hasn't been touched by pgindent since the 8.3 run. But the changes to it
all look fairly kosher at first glance.

Oh, I bet you're right -- see commit
97116ca4170b974d734cea364789c389b30e6ce1. That removed the exclusion
but I don't see any evidence that Bruce actually pgindent'd it then.

Right. The fix for that file was some time after the pgindent run and I
didn't want to run just that file.

This is the first time I ever had no pgindent failure/warning messages.

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

+ It's impossible for everything to be true. +