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.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 741b0ccb58..2834f5c9fc 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;
