On Fri Dec 15, 2023 at 8:18 AM CST, Jelte Fennema-Nio wrote:
This part of the first patch seems incorrect, i.e. same condition in
the if as elsif
-       if ($silent_diff)
+       if ($check)
+       {
+           print show_diff($source, $source_filename);
+           exit 2;
+       }
+       elsif ($check)
        {
            exit 2;
        }
Weird how I was able to screw that up so bad :). Thanks! Here is a v3.
--
Tristan Partin
Neon (https://neon.tech)
From ed5a44a1552c407719ac8c94603b51a7e72084f0 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Mon, 11 Dec 2023 17:34:17 -0600
Subject: [PATCH v3 1/3] Rename --silent-diff to --check in pgindent

--check should be a little bit more self-explanatory for people coming
from other similar formatter/linting tooling.
---
 src/tools/pgindent/pgindent | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index bce63d95da..ca5f8543a6 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -23,7 +23,7 @@ my $devnull = File::Spec->devnull;
 
 my ($typedefs_file, $typedef_str, @excludes,
 	$indent, $build, $show_diff,
-	$silent_diff, $help, @commits,);
+	$check, $help, @commits,);
 
 $help = 0;
 
@@ -35,13 +35,13 @@ my %options = (
 	"excludes=s" => \@excludes,
 	"indent=s" => \$indent,
 	"show-diff" => \$show_diff,
-	"silent-diff" => \$silent_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 have both --check and --show-diff")
+  if $check && $show_diff;
 
 usage("Cannot use --commit with command line file list")
   if (@commits && @ARGV);
@@ -324,7 +324,7 @@ Options:
 	--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
+	--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,7 +417,7 @@ foreach my $source_filename (@files)
 
 	if ($source ne $orig_source)
 	{
-		if ($silent_diff)
+		if ($check)
 		{
 			exit 2;
 		}
-- 
Tristan Partin
Neon (https://neon.tech)

From 370539a8a86fea310a8e33c7298786923039dd13 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Thu, 14 Dec 2023 10:36:05 -0600
Subject: [PATCH v3 2/3] Rename --show-diff to --diff in pgindent

--diff should be a little bit more self-explanatory for people coming
from other similar formatter/linting tooling.
---
 src/tools/pgindent/pgindent | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index ca5f8543a6..38a0222573 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -22,7 +22,7 @@ my $indent_opts =
 my $devnull = File::Spec->devnull;
 
 my ($typedefs_file, $typedef_str, @excludes,
-	$indent, $build, $show_diff,
+	$indent, $build, $diff,
 	$check, $help, @commits,);
 
 $help = 0;
@@ -34,14 +34,14 @@ my %options = (
 	"list-of-typedefs=s" => \$typedef_str,
 	"excludes=s" => \@excludes,
 	"indent=s" => \$indent,
-	"show-diff" => \$show_diff,
+	"diff" => \$diff,
 	"check" => \$check,);
 GetOptions(%options) || usage("bad command line argument");
 
 usage() if $help;
 
-usage("Cannot have both --check and --show-diff")
-  if $check && $show_diff;
+usage("Cannot have both --check and --diff")
+  if $check && $diff;
 
 usage("Cannot use --commit with command line file list")
   if (@commits && @ARGV);
@@ -294,7 +294,7 @@ sub run_indent
 	return $source;
 }
 
-sub show_diff
+sub diff
 {
 	my $indented = shift;
 	my $source_filename = shift;
@@ -323,7 +323,7 @@ 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
+	--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
@@ -421,9 +421,9 @@ foreach my $source_filename (@files)
 		{
 			exit 2;
 		}
-		elsif ($show_diff)
+		elsif ($diff)
 		{
-			print show_diff($source, $source_filename);
+			print diff($source, $source_filename);
 		}
 		else
 		{
-- 
Tristan Partin
Neon (https://neon.tech)

From 57ceebbc81f97da2e6ca28d8c145918732be5382 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Thu, 14 Dec 2023 10:39:32 -0600
Subject: [PATCH v3 3/3] Allow --check and --diff to passed in the same
 pgindent command

This means to print the diff and exit if a diff exists.
---
 src/tools/pgindent/pgindent | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 38a0222573..eb2f52f4b9 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -40,9 +40,6 @@ GetOptions(%options) || usage("bad command line argument");
 
 usage() if $help;
 
-usage("Cannot have both --check and --diff")
-  if $check && $diff;
-
 usage("Cannot use --commit with command line file list")
   if (@commits && @ARGV);
 
@@ -417,17 +414,21 @@ foreach my $source_filename (@files)
 
 	if ($source ne $orig_source)
 	{
-		if ($check)
-		{
-			exit 2;
-		}
-		elsif ($diff)
+		if (!$diff && !$check)
 		{
-			print 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;
+			}
 		}
 	}
 }
-- 
Tristan Partin
Neon (https://neon.tech)

Reply via email to