Re: [HACKERS] Preliminary results for proposed new pgindent implementation

* Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Robert Haas (robertmh...@gmail.com) wrote: > >> On Sat, Jun 17, 2017 at 5:41 PM, Peter Eisentraut > >> wrote: > >>> On 6/16/17 10:51, Tom Lane wrote: > So I'm back to

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Stephen Frost writes: > * Robert Haas (robertmh...@gmail.com) wrote: >> On Sat, Jun 17, 2017 at 5:41 PM, Peter Eisentraut >> wrote: >>> On 6/16/17 10:51, Tom Lane wrote: So I'm back to the position that we ought to stick the indent

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

* Robert Haas (robertmh...@gmail.com) wrote: > On Sat, Jun 17, 2017 at 5:41 PM, Peter Eisentraut > wrote: > > On 6/16/17 10:51, Tom Lane wrote: > >> So I'm back to the position that we ought to stick the indent > >> code under src/tools/ in our main repo. Is

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On Sat, Jun 17, 2017 at 5:41 PM, Peter Eisentraut wrote: > On 6/16/17 10:51, Tom Lane wrote: >> So I'm back to the position that we ought to stick the indent >> code under src/tools/ in our main repo. Is anyone really >> seriously against that? > > I think it

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Peter Eisentraut writes: > On 6/16/17 10:51, Tom Lane wrote: >> So I'm back to the position that we ought to stick the indent >> code under src/tools/ in our main repo. Is anyone really >> seriously against that? > I think it would be better to have it

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 2017-06-17 21:55, Tom Lane wrote: > I spent some time looking into this. I reverted your commits > 198457848ae5c86bec3336a9437dd5aa30f480c2 (Replace err.h functions with > standard C equivalents) and fb10acb040b90bdcbad09defd303363db29257d1 > (Remove inclusion of sys/cdefs.h) locally and tried

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 6/16/17 10:51, Tom Lane wrote: > So I'm back to the position that we ought to stick the indent > code under src/tools/ in our main repo. Is anyone really > seriously against that? I think it would be better to have it separate. Other than for reasons of principle and general modularity of

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

I wrote: > Piotr Stefaniak writes: >> There are also the "portability fixes" and they're the main problem. > Fair enough. I spent some time looking into this. I reverted your commits 198457848ae5c86bec3336a9437dd5aa30f480c2 (Replace err.h functions with standard C

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Robert Haas writes: > On Fri, Jun 16, 2017 at 10:51 AM, Tom Lane wrote: >> So I'm back to the position that we ought to stick the indent >> code under src/tools/ in our main repo. Is anyone really >> seriously against that? > Is it under the same

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On Fri, Jun 16, 2017 at 10:51 AM, Tom Lane wrote: > So I'm back to the position that we ought to stick the indent > code under src/tools/ in our main repo. Is anyone really > seriously against that? Is it under the same license as everything else? -- Robert Haas

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Piotr Stefaniak writes: > On 2017-06-17 00:02, Tom Lane wrote: >> What I'm testing with right now has just four differences from your repo: > There are also the "portability fixes" and they're the main problem. Fair enough. > I've simply removed things like

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 2017-06-17 00:02, Tom Lane wrote: > Piotr Stefaniak writes: >> On 2017-06-16 21:56, Tom Lane wrote: >>> Unless Piotr objects, I propose to add another switch to bsdindent >>> that selects this behavior, and then we can drop entab, removing >>> another impediment to

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Piotr Stefaniak writes: > On 2017-06-16 21:56, Tom Lane wrote: >> Unless Piotr objects, I propose to add another switch to bsdindent >> that selects this behavior, and then we can drop entab, removing >> another impediment to getting pgindent working. > I understand

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Piotr Stefaniak writes: > On 2017-06-16 20:11, Tom Lane wrote: >> I assume though that Piotr wants an option to preserve that behavior. >> I'm happy to write up a patch for bsdindent that adds a switch >> controlling this, but is there any rhyme or reason to the way

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 2017-06-16 20:11, Tom Lane wrote: > Andres Freund writes: >> On 2017-06-16 13:44:30 -0400, Bruce Momjian wrote: >>> Yes, it is all about <80 column output. The current pgindent does >>> everything possible to accomplish that --- the question is whether we >>> want uglier

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 2017-06-16 21:56, Tom Lane wrote: > One other thing I'd like to do while we're changing this stuff is > to get rid of the need for entab/detab. Right now, after doing > all the other work, my copy of pgindent is running the code through > detab and then entab so as to match the old decisions

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On Fri, Jun 16, 2017 at 03:56:47PM -0400, Tom Lane wrote: > can be. I managed to tweak bsdindent so that its output matches > what entab would do, by dint of the attached patch, which implements > the rule "use a space instead of a tab if the tab would only move > one column and we don't need

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

One other thing I'd like to do while we're changing this stuff is to get rid of the need for entab/detab. Right now, after doing all the other work, my copy of pgindent is running the code through detab and then entab so as to match the old decisions about how to represent whitespace (ie, as

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On Fri, Jun 16, 2017 at 11:54:06AM -0700, Andres Freund wrote: > On 2017-06-16 14:42:38 -0400, Bruce Momjian wrote: > > On Fri, Jun 16, 2017 at 02:23:00PM -0400, Tom Lane wrote: > > > Well, that's something we need to discuss. I originally argued for > > > back-patching the new rules, whatever

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 2017-06-16 14:42:38 -0400, Bruce Momjian wrote: > On Fri, Jun 16, 2017 at 02:23:00PM -0400, Tom Lane wrote: > > Well, that's something we need to discuss. I originally argued for > > back-patching the new rules, whatever they are (ie, run the new > > pgindent on the back branches whenever

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On Fri, Jun 16, 2017 at 02:23:00PM -0400, Tom Lane wrote: > Well, that's something we need to discuss. I originally argued for > back-patching the new rules, whatever they are (ie, run the new > pgindent on the back branches whenever we've agreed that the dust > has settled). But I'm starting to

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Andres Freund writes: > On 2017-06-16 13:34:01 -0400, Tom Lane wrote: >> I do intend to apply the diffs to HEAD in multiple steps, just to >> make them more reviewable. But I think we should probably absorb >> all the changes we want into v10, not leave some for later cycles.

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Andres Freund writes: > On 2017-06-16 13:44:30 -0400, Bruce Momjian wrote: >> Yes, it is all about <80 column output. The current pgindent does >> everything possible to accomplish that --- the question is whether we >> want uglier code to do it. > For me personally the

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 2017-06-16 13:34:01 -0400, Tom Lane wrote: > > I could live with both of these proposed > > changes, the selection of the changes you posted looks like it could be > > improved by code changes, but that's obviously a large amount of work. > > In the end, the only thing that fixes this sort of

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 2017-06-16 13:44:30 -0400, Bruce Momjian wrote: > On Fri, Jun 16, 2017 at 01:34:01PM -0400, Tom Lane wrote: > > > I could live with both of these proposed > > > changes, the selection of the changes you posted looks like it could be > > > improved by code changes, but that's obviously a large

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On Fri, Jun 16, 2017 at 01:34:01PM -0400, Tom Lane wrote: > > I could live with both of these proposed > > changes, the selection of the changes you posted looks like it could be > > improved by code changes, but that's obviously a large amount of work. > > In the end, the only thing that fixes

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Andres Freund writes: > I think the current logic is pretty horrible, primarily because it's so > hard to get to manually. Yes, I think that's really the big argument against it: no editor on the face of the planet will indent code that way to start with. > I could live with

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Hi, On 2017-06-16 13:10:50 -0400, Tom Lane wrote: > I experimented with disabling that logic and just always aligning > to the paren indentation. That fixes the weird cases with continued > string literals, but it also makes for a heck of a lot of other changes. > The full diff is too big to

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

There was some discussion upthread about how we'd like pgindent not to do weird things with string literals that wrap around the end of the line a little bit. I looked into that and found that it's actually a generic behavior for any line that's within parentheses: normally, such a line will get

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Peter Eisentraut writes: > On 5/19/17 13:31, Alvaro Herrera wrote: >> I favor having indent in a separate repository in our Git server, for >> these reasons > I am also in favor of that. >> 0. it's under our control (so we can change rules as we see fit) >> 1.

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Peter Eisentraut writes: > On 5/19/17 11:22, Tom Lane wrote: >> I certainly would rather that our version matched something that's under >> active maintenance someplace. But it seems like there are two good >> arguments for having a copy in our tree: > Is

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 5/19/17 11:22, Tom Lane wrote: > I certainly would rather that our version matched something that's under > active maintenance someplace. But it seems like there are two good > arguments for having a copy in our tree: Is pgindent going to be indented by pgindent? -- Peter Eisentraut

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 5/19/17 13:31, Alvaro Herrera wrote: > I favor having indent in a separate repository in our Git server, for > these reasons I am also in favor of that. > 0. it's under our control (so we can change rules as we see fit) > 1. we can have Piotr as a committer there > 2. we can use the same

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

I wrote: > What I was just looking at is the possibility of absorbing struct > tags ("xllist" in the above) as if they were typedef names. In > at least 95% of our usages, if a struct has a tag then the tag is > also the struct's typedef name. The reason this is interesting > is that it looks

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Tom Lane wrote: > Robert Haas writes: > > Yeah, but those advantages could also be gained by putting the > > pgindent tree on git.postgresql.org in a separate repository. Having > > it in the same repository as the actual PostgreSQL code is not > > required nor, in my

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Andres Freund writes: > On 2017-05-19 12:21:52 -0400, Robert Haas wrote: >> Yeah, but those advantages could also be gained by putting the >> pgindent tree on git.postgresql.org in a separate repository. Having >> it in the same repository as the actual PostgreSQL code is not

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Heikki Linnakangas writes: > On 05/19/2017 06:48 PM, Tom Lane wrote: >> That's going to catch a lot of things that are just variables, though. >> It might be all right as long as there was manual filtering after it. > At a quick glance, there are only a couple of them. This two

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Robert Haas writes: > On Fri, May 19, 2017 at 11:22 AM, Tom Lane wrote: >> I certainly would rather that our version matched something that's under >> active maintenance someplace. But it seems like there are two good >> arguments for having a copy in

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 2017-05-19 12:21:52 -0400, Robert Haas wrote: > On Fri, May 19, 2017 at 11:22 AM, Tom Lane wrote: > > I certainly would rather that our version matched something that's under > > active maintenance someplace. But it seems like there are two good > > arguments for having a

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On Fri, May 19, 2017 at 11:22 AM, Tom Lane wrote: > I certainly would rather that our version matched something that's under > active maintenance someplace. But it seems like there are two good > arguments for having a copy in our tree: > > * easy accessibility for PG

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 05/19/2017 06:48 PM, Tom Lane wrote: Heikki Linnakangas writes: You can get a pretty good typedefs list just by looking for the pattern "} ;". That's going to catch a lot of things that are just variables, though. It might be all right as long as there was manual

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On Fri, May 19, 2017 at 11:22:54AM -0400, Tom Lane wrote: > We've had reasonably decent luck with tracking the tzcode/tzdata packages > as local copies, so I feel like we're not taking on anything unreasonable > if our model is that we'll occasionally (not oftener than once per year) > update our

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Heikki Linnakangas writes: > You can get a pretty good typedefs list just by looking for the pattern > "} ;". That's going to catch a lot of things that are just variables, though. It might be all right as long as there was manual filtering after it.

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On 05/19/2017 06:05 PM, Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Robert Haas writes: On Thu, May 18, 2017 at 11:00 PM, Tom Lane wrote: The reason PGSSTrackLevel is "unrecognized" is that it's not in typedefs.list, which is a

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> Yes, moving the goalposts on ease-of-use is an important consideration >> here. What that says to me is that we ought to pull FreeBSD indent >> into our tree, and provide Makefile support that makes it easy for

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Tom, all, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Robert Haas writes: > > On Thu, May 18, 2017 at 11:00 PM, Tom Lane wrote: > >> The reason PGSSTrackLevel is "unrecognized" is that it's not in > >> typedefs.list, which is a deficiency in our

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Robert Haas writes: > On Thu, May 18, 2017 at 11:00 PM, Tom Lane wrote: >> The reason PGSSTrackLevel is "unrecognized" is that it's not in >> typedefs.list, which is a deficiency in our typedef-collection >> technology not in indent. (I believe the

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

On Thu, May 18, 2017 at 11:00 PM, Tom Lane wrote: > * Improvements in formatting around sizeof and related constructs, > for example: > > * Likewise, operators after casts work better than before: > > * Sane formatting of function typedefs, for example: > > * Non-typedef

Re: [HACKERS] Preliminary results for proposed new pgindent implementation

Tom, all, * Tom Lane (t...@sss.pgh.pa.us) wrote: > All in all, this looks pretty darn good from here, and I'm thinking > we should push forward on it. +1. Thanks! Stephen signature.asc Description: Digital signature