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 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 to reproduce pgindent's h

[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 Herrera wrote: > Robert Haas escribió: >> On Wed, Aug 12, 2009 at 12:27 AM, Tom Lane 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

[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 Lane 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 me for doi

[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 Lane wrote: > Robert Haas writes: >> On Tue, Aug 11, 2009 at 10:08 PM, Tom Lane wrote: >>> If that's not it, you'd need to mention details. > >> Well, one thing I've noticed is that when a function prototype wraps >> around to the next line, you often change t

[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 Lane wrote: > Robert Haas 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 those changes were made >> accident

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 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 gi

[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 th

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 Stark 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, you're taking li

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 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 an excuse to fid

[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 Stark 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 getting a series

[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 Lane 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 immediately after every c

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 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 har

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 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 th

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 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 > tabs. It is wildly

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 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
Andrew Dunstan 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 we need to

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 Dunstan 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 an

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 Dunstan 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 case, I t

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 af

[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 Lane 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 8.4 cycle, t