Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-14 Thread Tom Lane
Piotr Stefaniak writes: > I would like to go a bit further than that. I see that GNU indent > doesn't recognize -V, but prints its version if you use the option > --version. I wish to implement the same option for FreeBSD indent, but > that would force a change in how

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-14 Thread Piotr Stefaniak
On 2017-06-14 17:05, Bruce Momjian wrote: > On Wed, Jun 14, 2017 at 10:38:40AM -0400, Tom Lane wrote: >> btw, I was slightly amused to notice that this version still calls itself >> >> $ indent -V >> pg_bsd_indent 1.3 >> >> Don't know what you plan to do with that program name, but we certainly >>

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-14 Thread Piotr Stefaniak
On 2017-06-14 19:31, Tom Lane wrote: > Does that test case pass for you? No, I broke it recently. Sorry. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-14 Thread Tom Lane
Piotr Stefaniak writes: > On 2017-06-13 18:22, Tom Lane wrote: >> Also, I am wondering about the test cases under tests/. I do not >> see anything in the Makefile or elsewhere suggesting how those are >> to be used. It would sure be nice to have some quick

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-14 Thread Bruce Momjian
On Wed, Jun 14, 2017 at 10:38:40AM -0400, Tom Lane wrote: > btw, I was slightly amused to notice that this version still calls itself > > $ indent -V > pg_bsd_indent 1.3 > > Don't know what you plan to do with that program name, but we certainly > need a version number bump so that pgindent can

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-14 Thread Tom Lane
btw, I was slightly amused to notice that this version still calls itself $ indent -V pg_bsd_indent 1.3 Don't know what you plan to do with that program name, but we certainly need a version number bump so that pgindent can tell that it's got an up-to-date copy. 1.4? 2.0?

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-13 Thread Tom Lane
Piotr Stefaniak writes: > On 2017-06-13 22:23, Tom Lane wrote: >> I could not find any places where reverting this change made the >> results worse, so I'm unclear on why you made it. > I must admit I'm a bit confused about why it's not fixed yet, but I'll > have to

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-13 Thread Piotr Stefaniak
On 2017-06-13 22:23, Tom Lane wrote: > I could not find any places where reverting this change made the > results worse, so I'm unclear on why you made it. I must admit I'm a bit confused about why it's not fixed yet, but I'll have to analyze that later this week. But the idea was to convince

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-13 Thread Bruce Momjian
On Tue, Jun 13, 2017 at 05:00:31PM -0400, Tom Lane wrote: > Anyway, it is now time to fish or cut bait. I don't think we can wait > much longer to decide whether we're going to adopt this new indent > version for PG 10. I think we should. The floor is open for votes. Works for me. -- Bruce

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-13 Thread Piotr Stefaniak
On 2017-06-13 18:22, Tom Lane wrote: > The Makefile is still BSD-ish of course, but I think > we'll just agree to disagree there. For compiling indent under Linux I use bmake(1). I have no problem with including a Makefile for GNU Make in my repository. As I understand it, there will be a copy

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-13 Thread Tom Lane
I've now done a round of comparisons of results of our old indent with your current version. There's still one serious bug in the latter: it continues to misformat enum typedefs, for instance *** PG_FUNCTION_INFO_V1(pg_prewarm); *** 33,40 typedef enum {

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-13 Thread Tom Lane
Piotr Stefaniak writes: >> There's also the portability issues: __FBSDID() and bcopy() and >> [and err.h]. > I think that's fixed as well. I just finished some preliminary portability testing and things look much improved. The Makefile is still BSD-ish of course,

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-12 Thread Bruce Momjian
On Sun, Jun 11, 2017 at 09:14:36PM +, Piotr Stefaniak wrote: > I've never been too excited to improve indent and it's increasingly > challenging for me to force myself to work on it now, after I've > invested so much of my spare time into it. So please bear with me if > there are any errors.

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-06-11 Thread Piotr Stefaniak
>>> * the comments get formatted differently for -ts4 than -ts8 Still haven't put any thought into it, so I still don't know what to do here. >>> * extra spacing getting inserted for fairly long labels I think the fix is as easy as not producing the space. I committed that. >>> * some enum

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-22 Thread Tom Lane
Piotr Stefaniak writes: > On 2017-05-22 01:50, Tom Lane wrote: >> Being lazy, I just wiped my copy and re-cloned, but it still seems the >> same as before ... last commit on the pass3 branch is from Mar 4. >> What branch should I be paying attention to? > pass3 is

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-21 Thread Piotr Stefaniak
On 2017-05-22 01:50, Tom Lane wrote: > Being lazy, I just wiped my copy and re-cloned, but it still seems the > same as before ... last commit on the pass3 branch is from Mar 4. > What branch should I be paying attention to? Sorry for the trouble, this is because I interactively git-rebased it in

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-21 Thread Tom Lane
Piotr Stefaniak writes: >> * const unsigned(*TABLE_index)[2]; >> * OffsetNumber(*findChildPtr) (GinBtree, Page, BlockNumber, OffsetNumber); >> * an overlength comment line is simply busted altogether > Fixed and pushed to my github repository. I'm pretty confused by

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-21 Thread Piotr Stefaniak
> * const unsigned(*TABLE_index)[2]; > * OffsetNumber(*findChildPtr) (GinBtree, Page, BlockNumber, OffsetNumber); > * an overlength comment line is simply busted altogether Fixed and pushed to my github repository. Note that indent won't wrap long lines like links or paths anymore. But obviously

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-21 Thread Bruce Momjian
On Sun, May 21, 2017 at 10:12:20AM -0400, Tom Lane wrote: > Piotr Stefaniak writes: > > On 2017-05-21 03:00, Tom Lane wrote: > >> I wrote: > >>> Also, I found two places where an overlength comment line is simply busted > >>> altogether --- notice that a character is

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-21 Thread Tom Lane
Piotr Stefaniak writes: > On 2017-05-21 03:00, Tom Lane wrote: >> I wrote: >>> Also, I found two places where an overlength comment line is simply busted >>> altogether --- notice that a character is missing at the split point: >> I found the cause of that: you need

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-20 Thread Piotr Stefaniak
On 2017-05-21 03:00, Tom Lane wrote: > I wrote: >> Also, I found two places where an overlength comment line is simply busted >> altogether --- notice that a character is missing at the split point: > > I found the cause of that: you need to apply this patch: > > --- freebsd_indent/pr_comment.c~

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-20 Thread Tom Lane
I wrote: > Also, I found two places where an overlength comment line is simply busted > altogether --- notice that a character is missing at the split point: I found the cause of that: you need to apply this patch: --- freebsd_indent/pr_comment.c~2017-05-17 14:59:31.548442801 -0400 +++

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-18 Thread Tom Lane
I wrote: > Curiously, there are other enum declarations that don't get the phony > extra indentation. I traced through it a bit and eventually found that > the difference between OK and not OK is that the declarations that don't > get messed up look like "typedef enum enumlabel ...", ie the

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-18 Thread Tom Lane
Piotr Stefaniak writes: > On 2017-05-17 23:46, Tom Lane wrote: >> ... Much of what >> I'm seeing with this version is randomly different decisions about >> how far to indent comments > pgindent doesn't set the -c indent option ("The column in which comments > on code

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-18 Thread Piotr Stefaniak
On 2017-05-17 23:46, Tom Lane wrote: > I hacked around that by putting back a detab/entab step at the end > using the existing subroutines in pgindent. That about halved the > size of the diff, but it's still too big to post. Much of what > I'm seeing with this version is randomly different

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-17 Thread Tom Lane
Piotr Stefaniak writes: > Full copy of my pgindent attached. Changes commented below. Thanks! I ran this, along with the indent copy I pulled from your github repo a couple hours ago, over the current PG tree (post Bruce's run). I got a diff extending to about

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-17 Thread Piotr Stefaniak
On 2017-05-17 22:11, Tom Lane wrote: > Piotr Stefaniak writes: >> The third significant issue I kept in my mind was to shift some >> work-arounds from pgindent to indent. > > Yeah, I was wondering about that too. > >> When I use my indent as the base >> for pgindent

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-17 Thread Tom Lane
Piotr Stefaniak writes: > The third significant issue I kept in my mind was to shift some > work-arounds from pgindent to indent. Yeah, I was wondering about that too. > When I use my indent as the base > for pgindent and set its options like this: > -bad -bap -bc

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-17 Thread Piotr Stefaniak
On 2017-05-17 20:31, Tom Lane wrote: > Piotr Stefaniak writes: >> On 2017-05-17 19:16, Alvaro Herrera wrote: >>> We have someone who has studied the BSD indent code and even sent us >>> patches to fix quite a few bugs, but we've largely ignored his efforts >>> so far.

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-17 Thread Andres Freund
On 2017-05-17 14:44:31 -0400, Tom Lane wrote: > $ tar xfz ~/archive/pg_bsd_indent-1.3.tar.gz > $ wc pg_bsd_indent/* > 38122928 pg_bsd_indent/Makefile >107831 4835 pg_bsd_indent/README >508 1743 11988 pg_bsd_indent/args.c >569 2727 14732 pg_bsd_indent/indent.1 >

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-17 Thread Tom Lane
Andres Freund writes: > On 2017-05-17 13:35:22 -0400, Tom Lane wrote: >> Not sure about actually incorporating it into our repo. Doing so would >> make it easier for people to use, for sure, and the license seems to be >> regular 3-clause BSD, so that angle is OK. But do we

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-17 Thread Tom Lane
Alvaro Herrera writes: > Tom Lane wrote: >> Not sure about actually incorporating it into our repo. Doing so would >> make it easier for people to use, for sure, and the license seems to be >> regular 3-clause BSD, so that angle is OK. But do we want to be carrying >>

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-17 Thread Andres Freund
On 2017-05-17 13:35:22 -0400, Tom Lane wrote: > Not sure about actually incorporating it into our repo. Doing so would > make it easier for people to use, for sure, and the license seems to be > regular 3-clause BSD, so that angle is OK. But do we want to be carrying > around another 150K of

Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)

2017-05-17 Thread Alvaro Herrera
Tom Lane wrote: > Alvaro Herrera writes: > > We have someone who has studied the BSD indent code and even sent us > > patches to fix quite a few bugs, but we've largely ignored his efforts > > so far. I propose we take that indent as part of our repo, and patch it > >