Обсуждение: pgindent
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? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
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. +