[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 wraps around to the next line, you often change the number of spaces in the hanging indent. 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: static int myfunction(int foo, int bar) What is not obvious is that the same amount of hanging indent is used for the function's *prototype*, even though the first line is not the same: static int myfunction(int foo, int bar); ie, the indent is length of function name plus 1. I find this a bit stupid myself and would prefer that the prototypes looked like this: static int myfunction(int foo, int bar); That is truly bizarre. +1 from me for doing something that a competent C programmer can figure out without a calculator. I don't care what the rule is particularly, as long as it's obvious how to follow it. (In my own code I indent all of my continuation lines by one additional 4-space tab-stop. I realize this would be a horrible idea for PG since we don't want to change anything that's going to reindent the entire code base, and you might all hate it for other reasons anyway, but the point is that any idiot can look at it and figure out how it's supposed to be indented, because the rule is simple.) which is what my editor will do with it if left to its own devices. So depending on whether I had occasion to touch the prototype, you might see it get reindented to the second style. (I know pgindent will change it again, but I'm not sufficiently anal to override Emacs here.) Also, I tend to make sure that there are not so many parameters declared on the same line that it would look bad if pgindent ever starts doing things my way, ie, if I see static someveryverylongtypename *somefunctionname(int foo, int bar, int baz, int quux, int somethingelse, int more); I'll probably split the second line, even though there's room for it according to pgindent's current rule. So, yeah, I'm being a bit anal about this and yet I'm not always consistent. But that's what pgindent is good at fixing ... I guess all I can say on this point is it might be helpful to people if you avoid changing minor deviations from the pgindent standard unless you're pretty sure that you're matching the standard exactly. I am also a bit confused about the rule for indenting the variables in variable declarations. I get that the *, if any, is suppose to hang to the left of the name, so that the first character of each variable name lines up - and in particular, line up on a tab stop. But it's not obvious to me how to choose which tab stop to line everything up on. I'm not too sure either. It's the fourth tab stop for declarations at the outer level of a function, but I've never entirely figured out what pgindent's rule is for nested declarations. It seems to be willing to move the target column over to some extent, but eventually it gives up and doesn't add any more whitespace. I don't usually try very hard to align declarations exactly where pgindent will put them, although if it's visually ragged as-submitted sometimes I'll fix that. One thing I frequently *do* do is remove submitted whitespace changes if I know that pgindent would undo them anyway, just to reduce the size of the final diff. A case that happened at least once in this commitfest is that somebody had taken a block of variables aligned to the fourth tab stop: int foo; int bar; int baz; and realigned them to the fifth stop, because he was adding another variable with a longer type name: int foo; int bar; int baz; longtypename *ptr; Well, that looks nice, but pgindent is going to do this anyway: int foo; int bar; int baz; longtypename *ptr; so I tend to revert such dead-end whitespace changes just as part of making sure there wasn't an unintentional change of the existing lines. (This is the sort of thing that having submitters pgindent beforehand might avoid ...) Yeah, I probably did that, because I didn't understand the rules. Bored yet? No. This stuff really sucks. To give you another example of why it sucks, consider that I often rebase my patch on top of your commit to see what you changed, in an attempt to learn what I should do differently next time. Of course I get a conflict every place where you've made a change, which is the point, but the ones that are whitespace-only make a big mess that makes it a lot harder to see what has really changed. The
[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 me for doing something that a competent C programmer can figure out without a calculator. I don't care what the rule is particularly, as long as it's obvious how to follow it. (In my own code I indent all of my continuation lines by one additional 4-space tab-stop. I realize this would be a horrible idea for PG since we don't want to change anything that's going to reindent the entire code base, and you might all hate it for other reasons anyway, but the point is that any idiot can look at it and figure out how it's supposed to be indented, because the rule is simple.) 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. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 all the parameters to start in the same column: That is truly bizarre. +1 from me for doing something that a competent C programmer can figure out without a calculator. I don't care what the rule is particularly, as long as it's obvious how to follow it. (In my own code I indent all of my continuation lines by one additional 4-space tab-stop. I realize this would be a horrible idea for PG since we don't want to change anything that's going to reindent the entire code base, and you might all hate it for other reasons anyway, but the point is that any idiot can look at it and figure out how it's supposed to be indented, because the rule is simple.) 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. Function prototypes was what I was referring to. I think I understand how to reproduce it now, but it's still bizarre. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 to reproduce pgindent's handling of a prototype is: static int foo(int ..., bool ...); * temporarily break the line: static int foo(int ..., bool ...); * indent continuation lines to match (in Emacs this just means pressing tab on each line): static int foo(int ..., bool ...); * rejoin the first line static int foo(int ..., bool ...); It's a couple more keystrokes than letting Emacs do what it would like, but not exactly hard. 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
[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 8.4 cycle, the pgindent run was done something like 7 months after the start of the last CommitFest, by which time a fair number of patches had accumulated. Yeah, that's a fair point. Maybe we should institute a new policy that pgindent should happen immediately after close of the last commitfest in a cycle, instead of delaying until almost release time. 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. I'm not sure whether that would work out to a net positive or not. Presumably, it would mostly only break patches that had touched that part of the code, and therefore were likely to be broken anyway. But by the same token, since they're under active development, they're also more likely to have already been fixed. 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 after a longish delay would be a good idea. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 after a longish delay would be a good idea. Frankly, fixing up patch bitrot caused by pgindent is not terribly difficult in my experience - bitrot caused by code drift is a much harder problem (and yes, git fans, I know git can help with that). cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 case, I think running pgindent immediately after the last CommitFest rather than after a longish delay would be a good idea. Frankly, fixing up patch bitrot caused by pgindent is not terribly difficult in my experience - bitrot caused by code drift is a much harder problem (and yes, git fans, I know git can help with that). 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. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 practical. But in any case, I think running pgindent immediately after the last CommitFest rather than after a longish delay would be a good idea. Frankly, fixing up patch bitrot caused by pgindent is not terribly difficult in my experience - bitrot caused by code drift is a much harder problem (and yes, git fans, I know git can help with that). 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 address more directly. I just looked at what it did to the DATA lines - it seems to have changed 501 of them, and all the changes seem to be to do with tabbing. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 we need to address more directly. I just looked at what it did to the DATA lines - it seems to have changed 501 of them, and all the changes seem to be to do with tabbing. That's interesting --- the whitespace in those macros has always been wildly inconsistent, so I assumed pgindent wasn't touching them at all. I wonder what it thinks it's doing... 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
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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() lines is the pits. Oh? Maybe that's a problem we need to address more directly. I just looked at what it did to the DATA lines - it seems to have changed 501 of them, and all the changes seem to be to do with tabbing. That's interesting --- the whitespace in those macros has always been wildly inconsistent, so I assumed pgindent wasn't touching them at all. I wonder what it thinks it's doing... 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 undocumented. See src/tools/entab -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 tabs. It is wildly undocumented. See src/tools/entab What surprises me is that it seems to be changing lines that have been there for quite some time. Bruce, have you been changing pgindent underneath us? 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
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 replaces them with tabs. It is wildly undocumented. See src/tools/entab What surprises me is that it seems to be changing lines that have been there for quite some time. Bruce, have you been changing pgindent underneath us? Not unless I am CVS committing without showing anyone. ;-) These are the only commits I see recently, which are harmless: revision 1.101 date: 2009/06/11 22:21:44; author: momjian; state: Exp; lines: +7 -2 Document struct/union problem with pgindent. revision 1.100 date: 2008/11/03 15:56:47; author: momjian; state: Exp; lines: +2 -2 Small shell syntax improvement. revision 1.99 date: 2008/04/16 21:03:08; author: momjian; state: Exp; lines: +2 -2 Ignore blank lines in typedef file. revision 1.98 date: 2008/01/16 20:13:44; author: momjian; state: Exp; lines: +2 -2 Improve usage message for pgindent. revision 1.97 date: 2007/12/21 14:20:36; author: momjian; state: Exp; lines: +11 -2008 Modify pgindent to use an external typedefs file rather than included list. and entab is similarly unchanged: revision 1.18 date: 2007/02/08 11:10:27; author: petere; state: Exp; lines: +2 -2 Normalize fgets() calls to use sizeof() for calculating the buffer size where possible, and fix some sites that apparently thought that fgets() will overwrite the buffer by one byte. Also add some strlcpy() to eliminate some weird memory handling. I am stumped, but can keep researching. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 compare code, but comparing DATA() lines is the pits. Oh? Maybe that's a problem we need to address more directly. I just looked at what it did to the DATA lines - it seems to have changed 501 of them, and all the changes seem to be to do with tabbing. That's interesting --- the whitespace in those macros has always been wildly inconsistent, so I assumed pgindent wasn't touching them at all. I wonder what it thinks it's doing... 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 undocumented. See src/tools/entab I am not sure what documentation you want for it that isn't already there. There is an entab.man, and it is mentioned in the developer's FAQ, and it understands 'entab -h' for help. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 immediately after every commit? So nobody would ever see a checkout that wasn't pgindent-clean? The only losers I see would be people working on multi-part patches. If just one patch was committed they would have to resolve the conflicts in their subsequent patches before resubmitting. Of course in all likelihood tom would have rewritten their first patch anyways... -- greg http://mit.edu/~gsstark/resume.pdf -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 getting a series of four patches committed. I believe Tom changed the first two only a little. Tom asked me to make modifications that to the third one that were a lot of work but with which I fundamentally agreed, and made some modifications of his own to the fourth one which were improvements - minor, but improvements. 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 were made because Tom had some reason to believe that they would play more nicely with pgindent, though what those reasons may have been is entirely opaque to me. I think that it is good for us as a community to talk about the reasons why Tom changes patches. If some of them are bad reasons, maybe talking about it will persuade him to stop. If they are good reasons, perhaps the rest of us can learn from them. But I think it behooves us to talk about specific problems rather than engage in open-ended griping. I haven't been a member of this community for a super-long time, but already I can see that there is a correlation between who wrote the patch and how heavily it gets edited on commit. Of course, it's not 100%. Some of the thing that Tom wants are, at least AFAICS, things that Tom wants merely because he wants them, and not because they are better. That's not wonderful, but any of us would probably do the same thing, to some degree or other, if we were in his shoes. Magnus Hagander recently submitted a patch against my pgcommitfest application, and just look what I did to him: http://git.postgresql.org/gitweb?p=pgcommitfest.git;a=summary ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 were made because Tom had some reason to believe that they would play more nicely with pgindent, though what those reasons may have been is entirely opaque to me. I think that it is good for us as a community to talk about the reasons why Tom changes patches. If some of them are bad reasons, maybe talking about it will persuade him to stop. If they are good reasons, perhaps the rest of us can learn from them. But I think it behooves us to talk about specific problems rather than engage in open-ended griping. I haven't been a member of this community for a super-long time, but already I can see that there is a correlation between who wrote the patch and how heavily it gets edited on commit. Sometimes I clean up code style in patches because I don't know there are other patches depending on it; if I knew there were, I would be less likely to change patched code, but it is hard to remember which patches have others pending and which don't. Perhaps being explicit might help remind patch appliers. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 would give pgindent an excuse to fiddle with the whitespace here. And all it needs is an excuse. ;-) -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 an excuse to fiddle with the whitespace here. 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
Re: [HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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, you're taking life too seriously ;-) I took it that there was an implied smiley at the end of this remark. It certainly didn't strike me that it was snide. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: pgindent timing (was Re: [COMMITTERS] pgsql: Refactor NUM_cache_remove calls in error report path to a PG_TRY)
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 those changes were made accidentally as Tom was fooling around with what I had done, or they were made because Tom had some reason to believe that they would play more nicely with pgindent, though what those reasons may have been is entirely opaque to me. I tend to do some manual adjustment to pgindent rules in places where I feel it makes the code more readable. (Bear in mind that I've been looking at the Postgres code base for long enough that large variations from pgindent style are automatically less readable for me...) If I'd realized you had followon patches touching the same spots I would've possibly refrained from that. Although the nearby suggestions that we should run pgindent more often would render it moot ;-). 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 the number of spaces in the hanging indent. I would do them the way that you want them if I could figure out what the rule was, but I can't. When I asked before, you said that you let emacs do what it wants, but with apologies for using the Other Editor, that doesn't actually help me very much. I am also a bit confused about the rule for indenting the variables in variable declarations. I get that the *, if any, is suppose to hang to the left of the name, so that the first character of each variable name lines up - and in particular, line up on a tab stop. But it's not obvious to me how to choose which tab stop to line everything up on. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
