[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-12 Thread Robert Haas
On Wed, Aug 12, 2009 at 12:27 AM, Tom Lanet...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Tue, Aug 11, 2009 at 10:08 PM, Tom Lanet...@sss.pgh.pa.us wrote: If that's not it, you'd need to mention details. Well, one thing I've noticed is that when a function prototype

[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-12 Thread Alvaro Herrera
Robert Haas escribió: On Wed, Aug 12, 2009 at 12:27 AM, Tom Lanet...@sss.pgh.pa.us wrote: Ah.  That's a bit idiosyncratic to pgindent.  What it does for a function definition makes sense, I think: it lines up all the parameters to start in the same column: That is truly bizarre. +1 from

[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-12 Thread Robert Haas
On Wed, Aug 12, 2009 at 9:38 AM, Alvaro Herreraalvhe...@commandprompt.com wrote: Robert Haas escribió: On Wed, Aug 12, 2009 at 12:27 AM, Tom Lanet...@sss.pgh.pa.us wrote: Ah.  That's a bit idiosyncratic to pgindent.  What it does for a function definition makes sense, I think: it lines up

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-12 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes: Well, the rule here is simple too (set cinoptions=(0 if you're Vim-enabled). It's only function prototypes that are a bit weird, and once you understand how it works it's trivial to reproduce. Yeah. What I normally do if I'm actually trying

[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Robert Haas
On Tue, Aug 11, 2009 at 11:56 AM, Tom Lanet...@sss.pgh.pa.us wrote: OK, I get it.  Thanks for bearing with me.  The theory that the smallest amount of patches remain outstanding at that point is probably only true if the pgindent run is done relatively soon after the last CommitFest.  In the

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Andrew Dunstan
Robert Haas wrote: I'm not sure there's a good solution to this problem short of making pgindent easy enough that we can make it a requirement for patch submission, and I'm not sure that's practical. But in any case, I think running pgindent immediately after the last CommitFest rather than

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Robert Haas
On Tue, Aug 11, 2009 at 12:52 PM, Andrew Dunstanand...@dunslane.net wrote: Robert Haas wrote: I'm not sure there's a good solution to this problem short of making pgindent easy enough that we can make it a requirement for patch submission, and I'm not sure that's practical. But in any

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Andrew Dunstan
Robert Haas wrote: On Tue, Aug 11, 2009 at 12:52 PM, Andrew Dunstanand...@dunslane.net wrote: Robert Haas wrote: I'm not sure there's a good solution to this problem short of making pgindent easy enough that we can make it a requirement for patch submission, and I'm not sure that's

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Tom Lane
Andrew Dunstan and...@dunslane.net writes: Robert Haas wrote: Where it really bit me as when it reindented the DATA() statements that were touched by ALTER TABLE ... SET STATISTICS DISTINCT. It's not so hard to compare code, but comparing DATA() lines is the pits. Oh? Maybe that's a problem

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Alvaro Herrera
Andrew Dunstan escribió: Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Robert Haas wrote: Where it really bit me as when it reindented the DATA() statements that were touched by ALTER TABLE ... SET STATISTICS DISTINCT. It's not so hard to compare code, but comparing DATA()

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Tom Lane
Alvaro Herrera alvhe...@commandprompt.com writes: Andrew Dunstan escribió: Here's the extract attached. I replace tabs with a literal '\t' so I could see what it was doing. I can't make much head or tail of it either. pgindent uses entab/detab, which counts spaces and replaces them with

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Bruce Momjian
Tom Lane wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Andrew Dunstan escribi?: Here's the extract attached. I replace tabs with a literal '\t' so I could see what it was doing. I can't make much head or tail of it either. pgindent uses entab/detab, which counts spaces and

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Bruce Momjian
Alvaro Herrera wrote: Andrew Dunstan escribi?: Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: Robert Haas wrote: Where it really bit me as when it reindented the DATA() statements that were touched by ALTER TABLE ... SET STATISTICS DISTINCT. It's not so hard to

[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Greg Stark
On Tue, Aug 11, 2009 at 4:56 PM, Tom Lanet...@sss.pgh.pa.us wrote: A more aggressive approach would be to run pgindent immediately after the close of *each* commitfest, but that would tend to break patches that had gotten punted to the next fest. What would happen if we ran pgindent

[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Robert Haas
On Tue, Aug 11, 2009 at 8:10 PM, Greg Starkgsst...@mit.edu wrote: Of course in all likelihood tom would have rewritten their first patch anyways... Maybe I'm taking life too seriously at the moment, but I find this comment kind of snide and unhelpful. I just went through the experience of

[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Bruce Momjian
Robert Haas wrote: What is a bit frustrating to me is that a number of Tom's changes to the first two patches were trivial whitespace changes that required me to rebase for no obvious reason. Either those changes were made accidentally as Tom was fooling around with what I had done, or they

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Bruce Momjian
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: I compared 8.3 CVS against 8.4 CVS and see the removal of a column in pg_amop.h for exacly the lines you listed: Ah. The removal of amopreqcheck was so long ago I'd forgotten about it ;-) Yeah, the column additions/removals since 8.3

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Tom Lane
Bruce Momjian br...@momjian.us writes: I compared 8.3 CVS against 8.4 CVS and see the removal of a column in pg_amop.h for exacly the lines you listed: Ah. The removal of amopreqcheck was so long ago I'd forgotten about it ;-) Yeah, the column additions/removals since 8.3 would give pgindent

Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Andrew Dunstan
Robert Haas wrote: On Tue, Aug 11, 2009 at 8:10 PM, Greg Starkgsst...@mit.edu wrote: Of course in all likelihood tom would have rewritten their first patch anyways... Maybe I'm taking life too seriously at the moment, but I find this comment kind of snide and unhelpful. Yes,

[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)

2009-08-11 Thread Robert Haas
On Tue, Aug 11, 2009 at 10:08 PM, Tom Lanet...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: What is a bit frustrating to me is that a number of Tom's changes to the first two patches were trivial whitespace changes that required me to rebase for no obvious reason.   Either