Improve pgindent exclude handling: ignore empty lines
Hello,
We ran into an issue where pgindent stopped reformatting anything with
our custom exclude file, and after some investigation we found the
empty line accidentally inserted into the exclude file.
Pgindent currently treats empty lines as valid exclusions and creates
an empty regex from them. The empty regex matches any pattern,
resulting in ignoring everything.
As this behavior doesn't seem to be useful in practice, and it is easy
to reproduce accidentally (it works even at the end of the file), I
propose a patch that ignores empty lines in the exclude file.
If somebody really wants to ignore everything, that is still possible
with more explicit patterns like ".*"
Attachments:
0001-Improve-pgindent-exclude-handling-ignore-empty-lines.patchapplication/octet-stream; name=0001-Improve-pgindent-exclude-handling-ignore-empty-lines.patchDownload
From 7063f0c870d9929196d5af13d4804bf21e23f6f8 Mon Sep 17 00:00:00 2001
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Sat, 8 Feb 2025 09:29:43 +0000
Subject: [PATCH] Improve pgindent exclude handling: ignore empty lines
Pgindent currently only ignores lines starting with a "#", or
whitespace + "#", and treats everything else as a regex.
This also means "", the empty string, and the empty regex matches
everything.
As ignoring everything doesn't have any practical use, and can still
be done with a more explicit regex like ".*", this patch adds a
special handling to the empty line (or line consisting only of
whitespaces), to prevent exclude files that accidentally ignore
everything.
---
src/tools/pgindent/pgindent | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index d8acce7e929..36d86a0d3dd 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -182,7 +182,7 @@ sub process_exclude
while (my $line = <$eh>)
{
chomp $line;
- next if $line =~ m/^#/;
+ next if $line =~ m/^#/ || $line eq "";
my $rgx = qr!$line!;
@files = grep { $_ !~ /$rgx/ } @files if $rgx;
}
--
2.34.1
On 2025-02-08 Sa 4:39 AM, Zsolt Parragi wrote:
Hello,
We ran into an issue where pgindent stopped reformatting anything with
our custom exclude file, and after some investigation we found the
empty line accidentally inserted into the exclude file.Pgindent currently treats empty lines as valid exclusions and creates
an empty regex from them. The empty regex matches any pattern,
resulting in ignoring everything.As this behavior doesn't seem to be useful in practice, and it is easy
to reproduce accidentally (it works even at the end of the file), I
propose a patch that ignores empty lines in the exclude file.If somebody really wants to ignore everything, that is still possible
with more explicit patterns like ".*"
It seems reasonable to ignore an empty line. But your proposed commit
message isn't correct w.r.t. whitespace, either relating to current or
proposed behaviour. i.e. comment lines can't have leading whitespace,
and you patch won't ignore lines wconsisting of one or more whitespace
characters. So I'm not quite clear what you want to do.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Hello
You are right, when I was writing the commit message I was thinking
about trim, but the script only uses chomp.
I edited the commit message to correctly describe the new behavior:
ignoring only the empty line, as that was my main intention with the
patch.
Show quoted text
On Fri, Feb 14, 2025 at 3:24 PM Andrew Dunstan <andrew@dunslane.net> wrote:
On 2025-02-08 Sa 4:39 AM, Zsolt Parragi wrote:
Hello,
We ran into an issue where pgindent stopped reformatting anything with
our custom exclude file, and after some investigation we found the
empty line accidentally inserted into the exclude file.Pgindent currently treats empty lines as valid exclusions and creates
an empty regex from them. The empty regex matches any pattern,
resulting in ignoring everything.As this behavior doesn't seem to be useful in practice, and it is easy
to reproduce accidentally (it works even at the end of the file), I
propose a patch that ignores empty lines in the exclude file.If somebody really wants to ignore everything, that is still possible
with more explicit patterns like ".*"It seems reasonable to ignore an empty line. But your proposed commit
message isn't correct w.r.t. whitespace, either relating to current or
proposed behaviour. i.e. comment lines can't have leading whitespace,
and you patch won't ignore lines wconsisting of one or more whitespace
characters. So I'm not quite clear what you want to do.cheers
andrew
Attachments:
v2-0001-Improve-pgindent-exclude-handling-ignore-empty-lines.patchapplication/octet-stream; name=v2-0001-Improve-pgindent-exclude-handling-ignore-empty-lines.patchDownload
From aea18eb0112db767c3697a3bb90260414d894d84 Mon Sep 17 00:00:00 2001
From: Zsolt Parragi <zsolt.parragi@percona.com>
Date: Sat, 8 Feb 2025 09:29:43 +0000
Subject: [PATCH] Improve pgindent exclude handling: ignore empty lines
Pgindent currently only ignores lines starting with a "#" and
treats everything else as a regex.
This also means "", the empty string, and the empty regex matches
everything.
As ignoring everything doesn't have any practical use, and can still
be done with a more explicit regex like ".*", this patch adds a
special handling to the empty line to prevent exclude files that
accidentally ignore everything.
---
src/tools/pgindent/pgindent | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index d8acce7e929..36d86a0d3dd 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -182,7 +182,7 @@ sub process_exclude
while (my $line = <$eh>)
{
chomp $line;
- next if $line =~ m/^#/;
+ next if $line =~ m/^#/ || $line eq "";
my $rgx = qr!$line!;
@files = grep { $_ !~ /$rgx/ } @files if $rgx;
}
--
2.34.1
On 2025-02-18 Tu 3:02 PM, Zsolt Parragi wrote:
Hello
You are right, when I was writing the commit message I was thinking
about trim, but the script only uses chomp.I edited the commit message to correctly describe the new behavior:
ignoring only the empty line, as that was my main intention with the
patch.
pushed, thanks.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com