On 2023-01-21 Sa 10:02, Andrew Dunstan wrote: > On 2023-01-21 Sa 10:00, Andrew Dunstan wrote: >> On 2023-01-21 Sa 08:26, Andrew Dunstan wrote: >>> On 2023-01-20 Fr 13:19, Tom Lane wrote: >>>> Andres Freund <and...@anarazel.de> writes: >>>>> On 2023-01-20 12:09:05 -0500, Tom Lane wrote: >>>>>> The core problem here is that requiring that would translate to >>>>>> requiring every code contributor to have a working copy of pg_bsd_indent. >>>>> Wouldn't just every committer suffice? >>>> Not if we have cfbot complaining about it. >>>> >>>> (Another problem here is that there's a sizable subset of committers >>>> who clearly just don't care, and I'm not sure we can convince them to.) >>> I think we could do better with some automation tooling for committers >>> here. One low-risk and simple change would be to provide a >>> non-destructive mode for pgindent that would show you the changes if any >>> it would make. That could be worked into a git pre-commit hook that >>> committers could deploy. I can testify to the usefulness of such hooks - >>> I have one that while not perfect has saved me on at least two occasions >>> from forgetting to bump the catalog version. >>> >>> I'll take a look at fleshing this out, for my own if no-one else's use. >>> >>> >> Here's a quick patch for this. I have it in mind to use like this in a >> pre-commit hook: >> >> # only do this on master >> test `git rev-parse --abbrev-ref HEAD` = "master" || exit 0 >> >> src/tools/pgindent/pg_indent --silent `git diff --cached --name-only` || >> \ >> >> { echo "Need a pgindent run" >&2 ; exit 1; } >> >> >> The committer could then run >> >> src/tools/pgindent/pg_indent --show-diff `git diff --cached --name-only` >> >> to see what changes it thinks are needed. >> >> > This time with patch. > > ... with typo fixed.
cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index 741b0ccb58..af0ddd1944 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -21,7 +21,8 @@ my $indent_opts = my $devnull = File::Spec->devnull; -my ($typedefs_file, $typedef_str, $code_base, $excludes, $indent, $build); +my ($typedefs_file, $typedef_str, $code_base, $excludes, $indent, $build, + $show_diff, $silent_diff); my %options = ( "typedefs=s" => \$typedefs_file, @@ -29,9 +30,14 @@ my %options = ( "code-base=s" => \$code_base, "excludes=s" => \$excludes, "indent=s" => \$indent, - "build" => \$build,); + "build" => \$build, + "show-diff" => \$show_diff, + "silent_diff" => \$silent_diff,); GetOptions(%options) || die "bad command line argument\n"; +die "Cannot have both --silent-diff and --show-diff" + if $silent_diff && $show_diff; + run_build($code_base) if ($build); # command line option wins, then first non-option arg, @@ -283,30 +289,19 @@ sub run_indent } - -# for development diagnostics -sub diff +sub show_diff { - my $pre = shift; - my $post = shift; - my $flags = shift || ""; + my $indented = shift; + my $source_filename = shift; - print STDERR "running diff\n"; - - my $pre_fh = new File::Temp(TEMPLATE => "pgdiffbXXXXX"); my $post_fh = new File::Temp(TEMPLATE => "pgdiffaXXXXX"); - print $pre_fh $pre; - print $post_fh $post; + print $post_fh $indented; - $pre_fh->close(); $post_fh->close(); - system( "diff $flags " - . $pre_fh->filename . " " - . $post_fh->filename - . " >&2"); - return; + my $diff = `diff -u $source_filename $post_fh->filename 2>&1`; + return $diff; } @@ -404,6 +399,8 @@ push(@files, @ARGV); foreach my $source_filename (@files) { + # ignore anything that's not a .c or .h file + next unless $source_filename =~/\.[ch]$/; # Automatically ignore .c and .h files that correspond to a .y or .l # file. indent tends to get badly confused by Bison/flex output, @@ -429,7 +426,24 @@ foreach my $source_filename (@files) $source = post_indent($source, $source_filename); - write_source($source, $source_filename) if $source ne $orig_source; + if ($source ne $orig_source) + { + if ($silent_diff) + { + exit 2; + } + elsif ($show_diff) + { + print show_diff($source, $source_filename); + } + else + { + write_source($source, $source_filename); + } + } + } build_clean($code_base) if $build; + +exit 0;