On Mon Dec 18, 2023 at 10:50 AM CST, Tristan Partin wrote:
On Mon Dec 18, 2023 at 10:14 AM CST, Jelte Fennema-Nio wrote:
> On Mon, 18 Dec 2023 at 13:42, Daniel Gustafsson <dan...@yesql.se> wrote:
> > I think this is pretty much ready to go, the attached v4 squashes the
changes
> > and fixes the man-page which also needed an update. The referenced Wiki
page
> > will need an edit or two after this goes in, but that's easy enough.
>
> One thing I'm wondering: When both --check and --diff are passed,
> should pgindent still early exit with 2 on the first incorrectly
> formatted file? Or should it show diffs for all failing files? I'm
> leaning towards the latter making more sense.
Makes sense. Let me iterate on the squashed version of the patch.
Here is an additional patch which implements the behavior you described.
The first patch is just Daniel's squashed version of my patches.
--
Tristan Partin
Neon (https://neon.tech)
From 5cd4c5e9af0fc6e2e385b79111a6cca66df6cad9 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dan...@yesql.se>
Date: Mon, 18 Dec 2023 13:22:12 +0100
Subject: [PATCH v5 1/2] Rename non-destructive modes in pgindent
This renames --silent-diff to --check and --show-diff to --diff,
in order to make the options a little bit more self-explanatory
for developers used to similar formatters/linters. --check and
--diff are also allowed to be combined.
Author: Tristan Partin <tris...@neon.tech>
Discussion: https://postgr.es/m/cxlx2xyth9s6.140sc6y61v...@neon.tech
---
src/tools/pgindent/pgindent | 35 +++++++++++++++++----------------
src/tools/pgindent/pgindent.man | 12 +++++------
2 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index bce63d95da..eb2f52f4b9 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -22,8 +22,8 @@ my $indent_opts =
my $devnull = File::Spec->devnull;
my ($typedefs_file, $typedef_str, @excludes,
- $indent, $build, $show_diff,
- $silent_diff, $help, @commits,);
+ $indent, $build, $diff,
+ $check, $help, @commits,);
$help = 0;
@@ -34,15 +34,12 @@ my %options = (
"list-of-typedefs=s" => \$typedef_str,
"excludes=s" => \@excludes,
"indent=s" => \$indent,
- "show-diff" => \$show_diff,
- "silent-diff" => \$silent_diff,);
+ "diff" => \$diff,
+ "check" => \$check,);
GetOptions(%options) || usage("bad command line argument");
usage() if $help;
-usage("Cannot have both --silent-diff and --show-diff")
- if $silent_diff && $show_diff;
-
usage("Cannot use --commit with command line file list")
if (@commits && @ARGV);
@@ -294,7 +291,7 @@ sub run_indent
return $source;
}
-sub show_diff
+sub diff
{
my $indented = shift;
my $source_filename = shift;
@@ -323,8 +320,8 @@ Options:
--list-of-typedefs=STR string containing typedefs, space separated
--excludes=PATH file containing list of filename patterns to ignore
--indent=PATH path to pg_bsd_indent program
- --show-diff show the changes that would be made
- --silent-diff exit with status 2 if any changes would be made
+ --diff show the changes that would be made
+ --check exit with status 2 if any changes would be made
The --excludes and --commit options can be given more than once.
EOF
if ($help)
@@ -417,17 +414,21 @@ foreach my $source_filename (@files)
if ($source ne $orig_source)
{
- if ($silent_diff)
- {
- exit 2;
- }
- elsif ($show_diff)
+ if (!$diff && !$check)
{
- print show_diff($source, $source_filename);
+ write_source($source, $source_filename);
}
else
{
- write_source($source, $source_filename);
+ if ($diff)
+ {
+ print diff($source, $source_filename);
+ }
+
+ if ($check)
+ {
+ exit 2;
+ }
}
}
}
diff --git a/src/tools/pgindent/pgindent.man b/src/tools/pgindent/pgindent.man
index fe411ee699..caab5cde91 100644
--- a/src/tools/pgindent/pgindent.man
+++ b/src/tools/pgindent/pgindent.man
@@ -31,13 +31,13 @@ find the file src/tools/pgindent/exclude_file_patterns. The --excludes option
can be used more than once to specify multiple files containing exclusion
patterns.
-There are also two non-destructive modes of pgindent. If given the --show-diff
+There are also two non-destructive modes of pgindent. If given the --diff
option pgindent will show the changes it would make, but doesn't actually make
-them. If given instead the --silent-diff option, pgindent will exit with a
-status of 2 if it finds any indent changes are required, but will not
-make the changes or give any other information. This mode is intended for
-possible use in a git pre-commit hook. An example of its use in a git hook
-can be seen at https://wiki.postgresql.org/wiki/Working_with_Git#Using_git_hooks
+them. If given instead the --check option, pgindent will exit with a status of
+2 if it finds any indent changes are required, but will not make the changes.
+This mode is intended for possible use in a git pre-commit hook. The --check
+and --diff options can be combined. An example of its use in a git hook can be
+seen at https://wiki.postgresql.org/wiki/Working_with_Git#Using_git_hooks
Any non-option arguments are taken as the names of files to be indented. In this
case only these files will be changed, and nothing else will be touched.
--
Tristan Partin
Neon (https://neon.tech)
From 131c44536c83c01097de9253f7b09bdd7f2c2b4c Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Mon, 18 Dec 2023 15:14:56 -0600
Subject: [PATCH v5 2/2] Print all diffs with pgindent --check
Previously, only one diff was printed if multiple files were out of
compliance.
---
src/tools/pgindent/pgindent | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index eb2f52f4b9..0ae4dcddb1 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -372,6 +372,7 @@ warn "No files to process" unless @files;
process_exclude();
my %processed;
+my $status = 0;
foreach my $source_filename (@files)
{
@@ -427,10 +428,10 @@ foreach my $source_filename (@files)
if ($check)
{
- exit 2;
+ $status = 2;
}
}
}
}
-exit 0;
+exit $status;
--
Tristan Partin
Neon (https://neon.tech)
