Обсуждение: pgindent vs. pgperltidy command-line arguments
Until PG15, calling pgindent without arguments would process the whole tree. Now you get No files to process at ./src/tools/pgindent/pgindent line 372. Is that intentional? Also, pgperltidy accepts no arguments and always processes the whole tree. It would be nice if there were a way to process individual files or directories, like pgindent can. Attached is a patch for this. (It seems that it works ok to pass regular files (not directories) to "find", but I'm not sure if it's portable.)
Вложения
> On 25 May 2023, at 11:10, Peter Eisentraut <peter@eisentraut.org> wrote: > Also, pgperltidy accepts no arguments and always processes the whole tree. It would be nice if there were a way to processindividual files or directories, like pgindent can. +1, thanks! I've wanted that several times but never gotten around to doing anything about it. -- Daniel Gustafsson
Peter Eisentraut <peter@eisentraut.org> writes:
> Until PG15, calling pgindent without arguments would process the whole
> tree. Now you get
> No files to process at ./src/tools/pgindent/pgindent line 372.
> Is that intentional?
It was intentional, cf b16259b3c and the linked discussion.
> Also, pgperltidy accepts no arguments and always processes the whole
> tree. It would be nice if there were a way to process individual files
> or directories, like pgindent can.
+1, although I wonder if we shouldn't follow pgindent's new lead
and require some argument(s).
> Attached is a patch for this.
> (It seems that it works ok to pass regular files (not directories) to
> "find", but I'm not sure if it's portable.)
The POSIX spec for find(1) gives an example of applying find to
what they evidently intend to be a plain file:
if [ -n "$(find file1 -prune -newer file2)" ]; then
printf %s\\n "file1 is newer than file2"
fi
So while I don't see it written in so many words, I think you
can assume it's portable.
regards, tom lane
On 25.05.23 15:20, Tom Lane wrote: > Peter Eisentraut <peter@eisentraut.org> writes: >> Until PG15, calling pgindent without arguments would process the whole >> tree. Now you get >> No files to process at ./src/tools/pgindent/pgindent line 372. >> Is that intentional? > > It was intentional, cf b16259b3c and the linked discussion. > >> Also, pgperltidy accepts no arguments and always processes the whole >> tree. It would be nice if there were a way to process individual files >> or directories, like pgindent can. > > +1, although I wonder if we shouldn't follow pgindent's new lead > and require some argument(s). That makes sense to me. Here is a small update with this behavior change and associated documentation update.
Вложения
On 2023-06-14 We 03:37, Peter Eisentraut wrote:
On 25.05.23 15:20, Tom Lane wrote:Peter Eisentraut <peter@eisentraut.org> writes:Until PG15, calling pgindent without arguments would process the whole
tree. Now you get
No files to process at ./src/tools/pgindent/pgindent line 372.
Is that intentional?
It was intentional, cf b16259b3c and the linked discussion.Also, pgperltidy accepts no arguments and always processes the whole
tree. It would be nice if there were a way to process individual files
or directories, like pgindent can.
+1, although I wonder if we shouldn't follow pgindent's new lead
and require some argument(s).
That makes sense to me. Here is a small update with this behavior change and associated documentation update.
I'm intending to add some of the new pgindent features to pgperltidy. Preparatory to that here's a rewrite of pgperltidy in perl - no new features yet but it does remove the hardcoded path, and requires you to pass in one or more files / directories as arguments.
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
Вложения
Andrew Dunstan <andrew@dunslane.net> writes:
> I'm intending to add some of the new pgindent features to
> pgperltidy. Preparatory to that here's a rewrite of pgperltidy in perl -
> no new features yet but it does remove the hardcoded path, and requires
> you to pass in one or more files / directories as arguments.
Good idea, here's some comments.
> #!/usr/bin/perl
>
> # Copyright (c) 2023, PostgreSQL Global Development Group
>
> # src/tools/pgindent/pgperltidy
>
> use strict;
> use warnings;
>
> use File::Find;
>
> my $perltidy = $ENV{PERLTIDY} || 'perltidy';
>
> my @files;
>
> die "No directories or files specified" unless @ARGV;
It's not really useful to have the file name and line in errors like
this, adding a "\n" to the end of the message suppresses that.
> sub is_perl_exec
> {
> my $name = shift;
> my $out = `file $name 2>/dev/null`;
> return $out =~ /:.*perl[0-9]*\b/i;
> }
> my $wanted = sub {
>
> my $name = $File::Find::name;
> my ($dev, $ino, $mode, $nlink, $uid, $gid);
>
> # check it's a plain file and either it has a perl extension (.p[lm])
> # or it's executable and `file` thinks it's a perl script.
>
> (($dev, $ino, $mode, $nlink, $uid, $gid) = lstat($_))
> && -f _
> && (/\.p[lm]$/ || ((($mode & 0100) == 0100) && is_perl_exec($_)))
> && push(@files, $name);
> };
The core File::stat and Fcntl modules can make this neater:
use File::stat;
use Fcntl ':mode';
my $wanted = sub {
my $st;
push @files, $File::Find::name
if $st = lstat($_) && -f $st
&& (/\.p[lm]$/ || (($st->mode & S_IXUSR) && is_perl_exec($_)));
};
> File::Find::find({ wanted => $wanted }, @ARGV);
>
> my $list = join(" ", @files);
>
> system "$perltidy --profile=src/tools/pgindent/perltidyrc $list";
It's better to use the list form of system, to avoid shell escaping
issues. Also, since this is the last thing in the script we might as
well exec it instead:
exec $perltidy, '--profile=src/tools/pgindent/perltidyrc', @files;
- ilmari
On 20.06.23 17:38, Andrew Dunstan wrote: >>> +1, although I wonder if we shouldn't follow pgindent's new lead >>> and require some argument(s). >> >> That makes sense to me. Here is a small update with this behavior >> change and associated documentation update. > > I'm intending to add some of the new pgindent features to pgperltidy. > Preparatory to that here's a rewrite of pgperltidy in perl - no new > features yet but it does remove the hardcoded path, and requires you to > pass in one or more files / directories as arguments. Are you planning to touch pgperlcritic and pgperlsyncheck as well? If not, part of my patch would still be useful. Maybe I should commit my posted patch for PG16, to keep consistency with pgindent, and then your work would presumably be considered for PG17.
On 2023-06-21 We 05:09, Peter Eisentraut wrote:
On 20.06.23 17:38, Andrew Dunstan wrote:+1, although I wonder if we shouldn't follow pgindent's new lead
and require some argument(s).
That makes sense to me. Here is a small update with this behavior change and associated documentation update.
I'm intending to add some of the new pgindent features to pgperltidy. Preparatory to that here's a rewrite of pgperltidy in perl - no new features yet but it does remove the hardcoded path, and requires you to pass in one or more files / directories as arguments.
Are you planning to touch pgperlcritic and pgperlsyncheck as well?
Yeah, it would make sense to.
If not, part of my patch would still be useful. Maybe I should commit my posted patch for PG16, to keep consistency with pgindent, and then your work would presumably be considered for PG17.
That sounds like a good plan.
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com
On 21.06.23 13:35, Andrew Dunstan wrote: >> If not, part of my patch would still be useful. Maybe I should commit >> my posted patch for PG16, to keep consistency with pgindent, and then >> your work would presumably be considered for PG17. > > That sounds like a good plan. done
On 2023-06-21 We 07:35, Andrew Dunstan wrote:
On 2023-06-21 We 05:09, Peter Eisentraut wrote:On 20.06.23 17:38, Andrew Dunstan wrote:+1, although I wonder if we shouldn't follow pgindent's new lead
and require some argument(s).
That makes sense to me. Here is a small update with this behavior change and associated documentation update.
I'm intending to add some of the new pgindent features to pgperltidy. Preparatory to that here's a rewrite of pgperltidy in perl - no new features yet but it does remove the hardcoded path, and requires you to pass in one or more files / directories as arguments.
Are you planning to touch pgperlcritic and pgperlsyncheck as well?
Yeah, it would make sense to.
Here's a patch that turns all these into perl scripts.
cheers
andrew
-- Andrew Dunstan EDB: https://www.enterprisedb.com