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

On 2017-05-17 22:11, Tom Lane wrote:
> Piotr Stefaniak <postgres(at)piotr-stefaniak(dot)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(at)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.