Hi Andrew,
Thanks for the quick review.

On Wed, Jun 26, 2024 at 4:53 PM Andrew Dunstan <and...@dunslane.net> wrote:

>
> With --check option pgindent reports a non-zero exit code instead of
> making changes. So I feel the above change should happen at least if
> --check is provided. But certainly better if we do it even without --check.
>
> In case --check is specified and both the following happen a.
> pg_bsd_indent exits with non-zero status while processing some file and b.
> changes are produced while processing some other file, the program will
> exit with status 2. It may be argued that instead it should exit with code
> 3. I am open to both.
>
>
> Yeah, I think this is reasonable but we should adjust the status setting a
> few lines lower to
>
>
>    $status ||= 2;
>

So you are suggesting that status 3 is preferred over status 2 when both
are applicable. I am fine with that.

Here's what the behaviour looks like: (notice echo $? after running
pgindent)

1. Running without --check, if pgindent encounters file processing errors,
exit code is 3.
2. Running with --check, exit code depends upon whether pgindent encounters
a file with processing error first or a file that undergoes a change.
   a. If it encounters a file that would undergo a change first, exit
status is 2
   b. If it encounters a file with processing error first, exit status is 3
3. If --check is specified and no file undergoes a change, but there are
file processing errors, it will exit with code 3.

The variation in the second case based on the order of files processed
looks fine to me. What do you say?

The usage help mentions exit code 2 specifically while describing --check
option but it doesn't mention exit code 1. Neither does the README. So I
don't think we need to document exit code 3 anywhere. Please let me know if
you think otherwise.

-- 
Best Wishes,
Ashutosh Bapat
From 7a0f65a2367885eea0c9b7a4257e6faf758f58eb Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Wed, 26 Jun 2024 15:46:53 +0530
Subject: [PATCH 1/2] pgindent exit status on error

When pg_bsd_indent exits with a non-zero status or reports an error,
make pgindent exit with non-zero status 3. The program does not exit on
the first instance of the error. Instead it continues to process
remaining files as long as some other exit condition is encountered, in
which case exit code 3 is reported.

This helps to detect errors automatically when pgindent is run in shells
which interpret non-zero exit status as failure.

Author: Ashutosh Bapat
Reviewed by: Andrew Dunstan
Discussion: https://www.postgresql.org/message-id/caexhw5sprsifeldp-u1fa5ba7ys2f0gvljmkoobopkadjwq...@mail.gmail.com
---
 src/tools/pgindent/pgindent | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 48d83bc434..c9a8fd6561 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -409,6 +409,7 @@ foreach my $source_filename (@files)
 	if ($source eq "")
 	{
 		print STDERR "Failure in $source_filename: " . $error_message . "\n";
+		$status = 3;
 		next;
 	}
 
@@ -429,7 +430,7 @@ foreach my $source_filename (@files)
 
 			if ($check)
 			{
-				$status = 2;
+				$status ||= 2;
 				last unless $diff;
 			}
 		}
-- 
2.34.1

Reply via email to