On Wed Dec 13, 2023 at 2:46 PM CST, Andrew Dunstan wrote:
On 2023-12-12 Tu 10:30, Alvaro Herrera wrote:
> On 2023-Dec-12, Tom Lane wrote:
>
>> "Euler Taveira" <eu...@eulerto.com> writes:
>>> When you add exceptions, it starts to complicate the UI.
>> Indeed. It seems like --silent-diff was poorly defined and poorly
>> named, and we need to rethink that option along the way to adding
>> this behavior. The idea that --show-diff and --silent-diff can
>> be used together is just inherently confusing, because they sound
>> like opposites
> Maybe it's enough to rename --silent-diff to --check. You can do
> "--show-diff --check" and get both the error and the diff printed; or
> just "--check" and it'll throw an error without further ado; or
> "--show-diff" and it will both apply the diff and print it.
>
That seems reasonable. These features were fairly substantially debated
when we put them in, but I'm fine with some tweaking. But note:
--show-diff doesn't apply the diff, it's intentionally non-destructive.
Here is a new patch:
- Renames --silent-diff to --check
- Renames --show-diff to --diff
- Allows one to use --check and --diff in the same command
I am not tied to the second patch if people like --show-diff better than
--diff.
Weirdly enough, my email client doesn't show this as part of the
original thread, but I will respond here anyway.
--
Tristan Partin
Neon (https://neon.tech)
From f0963daa95b222b00a7ae15d6702141352d3a81d Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Mon, 11 Dec 2023 17:34:17 -0600
Subject: [PATCH v2 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 | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index bce63d95da..fe0bd21f4a 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,12 @@ foreach my $source_filename (@files)
if ($source ne $orig_source)
{
- if ($silent_diff)
+ if ($check)
+ {
+ print show_diff($source, $source_filename);
+ exit 2;
+ }
+ elsif ($check)
{
exit 2;
}
--
Tristan Partin
Neon (https://neon.tech)
From ddd5cdaf8e967ddf84beb49ca64b5bed039b71ac Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Thu, 14 Dec 2023 10:36:05 -0600
Subject: [PATCH v2 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 | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index fe0bd21f4a..067b77be54 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
@@ -419,16 +419,16 @@ foreach my $source_filename (@files)
{
if ($check)
{
- print show_diff($source, $source_filename);
+ print diff($source, $source_filename);
exit 2;
}
elsif ($check)
{
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 936fb5af395dbbc5fca56ac3a2eaae8216992159 Mon Sep 17 00:00:00 2001
From: Tristan Partin <tris...@neon.tech>
Date: Thu, 14 Dec 2023 10:39:32 -0600
Subject: [PATCH v2 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 | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 067b77be54..f69923fb00 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,22 +414,21 @@ foreach my $source_filename (@files)
if ($source ne $orig_source)
{
- if ($check)
- {
- print diff($source, $source_filename);
- exit 2;
- }
- elsif ($check)
+ if (!$show_diff && !$check)
{
- exit 2;
- }
- elsif ($diff)
- {
- print diff($source, $source_filename);
+ write_source($source, $source_filename);
}
else
{
- write_source($source, $source_filename);
+ if ($show_diff)
+ {
+ print diff($source, $source_filename);
+ }
+
+ if ($check)
+ {
+ exit 2;
+ }
}
}
}
--
Tristan Partin
Neon (https://neon.tech)
