Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventivemaintenance in advance of pgindent run.) - Mailing list pgsql-hackers

From Piotr Stefaniak
Subject Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventivemaintenance in advance of pgindent run.)
Date
Msg-id VI1PR03MB11999517FAD86D2C02407272F2E70@VI1PR03MB1199.eurprd03.prod.outlook.com
Whole thread Raw
In response to Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On 2017-05-17 22:11, Tom Lane wrote:
> Piotr Stefaniak <postgres@piotr-stefaniak.me> 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 -bl -d0 -cdb -nce -nfc1 -di12 -i4 -l79 -lp -nip -npro -bbb
>> -cli1 -sac -ts4 -cp10
>
> Ah, thanks, ignore the message I just sent asking for that ...
>
>> I can remove most of the work-arounds written in the perl script and
>> still get pretty decent results. But I expect some debate over a few things.
>
> ... but what parts of the perl script do you remove?  I'm trying
> to duplicate whatever results you're currently getting.

Full copy of my pgindent attached.  Changes commented below.


@@ -17,7 +17,7 @@ my $devnull        = File::Spec->devnull;

 # Common indent settings
 my $indent_opts =
-  "-bad -bap -bc -bl -d0 -cdb -nce -nfc1 -di12 -i4 -l79 -lp -nip -npro
-bbb";
+  "-bad -bap -bc -bl -d0 -cdb -nce -nfc1 -di12 -i4 -l79 -lp -nip -npro
-bbb -cli1 -sac -ts4 -cp10";

 # indent-dependent settings
 my $extra_opts = "";

The additional options are necessary. Mostly they replace the work-arounds.


@@ -198,60 +198,16 @@ sub pre_indent
 {
        my $source = shift;

-       # remove trailing whitespace
-       $source =~ s/[ \t]+$//gm;
-

I'm not sure there won't be any trailing white-space, but I've committed
changes that should limit it.


        ## Comments

        # Convert // comments to /* */
        $source =~ s!^([ \t]*)//(.*)$!$1/* $2 */!gm;

-       # 'else' followed by a single-line comment, followed by
-       # a brace on the next line confuses BSD indent, so we push
-       # the comment down to the next line, then later pull it
-       # back up again.  Add space before _PGMV or indent will add
-       # it for us.
-       # AMD: A symptom of not getting this right is that you see
errors like:
-       # FILE: ../../../src/backend/rewrite/rewriteHandler.c
-       # Error@2259:
-       # Stuff missing from end of file
-       $source =~
-         s!(\}|[ \t])else[ \t]*(/\*)(.*\*/)[ \t]*$!$1else\n    $2
_PGMV$3!gm;
-
-       # Indent multi-line after-'else' comment so BSD indent will move it
-       # properly. We already moved down single-line comments above.
-       # Check for '*' to make sure we are not in a single-line comment
that
-       # has other text on the line.
-       $source =~ s!(\}|[ \t])else[ \t]*(/\*[^*]*)[ \t]*$!$1else\n
$2!gm;

These are definitely fixed.


        ## Other

-       # Work around bug where function that defines no local variables
-       # misindents switch() case lines and line after #else.  Do not do
-       # for struct/enum.
-       my @srclines = split(/\n/, $source);
-       foreach my $lno (1 .. $#srclines)
-       {
-               my $l2 = $srclines[$lno];
-
-               # Line is only a single open brace in column 0
-               next unless $l2 =~ /^\{[ \t]*$/;
-
-               # previous line has a closing paren
-               next unless $srclines[ $lno - 1 ] =~ /\)/;
-
-               # previous line was struct, etc.
-               next
-                 if $srclines[ $lno - 1 ] =~
-                         m!=|^(struct|enum|[ \t]*typedef|extern[ \t]+"C")!;
-
-               $srclines[$lno] = "$l2\nint pgindent_func_no_var_fix;";
-       }
-       $source = join("\n", @srclines) . "\n";    # make sure there's a
final \n
-

I don't have time now to double-check, but the above shouldn't be needed
anymore.


-       # Pull up single-line comment after 'else' that was pulled down
above
-       $source =~ s!else\n[ \t]+/\* _PGMV!else\t/*!g;
-
-       # Indent single-line after-'else' comment by only one tab.
-       $source =~ s!(\}|[ \t])else[ \t]+(/\*.*\*/)[ \t]*$!$1else\t$2!gm;
-
-       # Add tab before comments with no whitespace before them (on a
tab stop)
-       $source =~ s!(\S)(/\*.*\*/)$!$1\t$2!gm;
-
-       # Remove blank line between opening brace and block comment.
-       $source =~ s!(\t*\{\n)\n([ \t]+/\*)$!$1$2!gm;
-

These are almost definitely fixed in indent.


-       # cpp conditionals
-
-       # Reduce whitespace between #endif and comments to one tab
-       $source =~ s!^\#endif[ \t]+/\*!#endif   /*!gm;
-
        ## Functions

-       # Work around misindenting of function with no variables defined.
-       $source =~ s!^[ \t]*int[ \t]+pgindent_func_no_var_fix;[
\t]*\n{1,2}!!gm;
-

These have likely been fixed.


-       ## Other
-
-       # Remove too much indenting after closing brace.
-       $source =~ s!^\}\t[ \t]+!}\t!gm;
-
-       # Workaround indent bug that places excessive space before 'static'.
-       $source =~ s!^static[ \t]+!static !gm;
-
-       # Remove leading whitespace from typedefs
-       $source =~ s!^[ \t]+typedef enum!typedef enum!gm
-         if $source_filename =~ 'libpq-(fe|events).h$';

I believe these have been fixed as well.


-       # Remove trailing blank lines
-       $source =~ s!\n+\z!\n!

I'm not sure, but shouldn't be necessary.


@@ -541,7 +462,6 @@ foreach my $source_filename (@files)

        # Protect backslashes in DATA() and wrapping in CATALOG()

-       $source = detab($source);
        $source =~ s!^((DATA|CATALOG)\(.*)$!/*$1*/!gm;

        $source = run_indent($source, \$error_message);
@@ -553,7 +473,6 @@ foreach my $source_filename (@files)

  # Restore DATA/CATALOG lines; must be done here so tab alignment is
preserved
        $source =~ s!^/\*((DATA|CATALOG)\(.*)\*/$!$1!gm;
-       $source = entab($source);

Definitely unnecessary if you use indent -ts4. There are slight differences.


Sorry for the delay, I was unprepared for answering this tonight.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: pgindent (was Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advance of pgindent run.)
Next
From: Piotr Stefaniak
Date:
Subject: Re: [HACKERS] [COMMITTERS] pgsql: Preventive maintenance in advanceof pgindent run.