Horton

Thread

Commits

  1. Allow and require passing files on command line of pgperltidy

  1. pgindent vs. pgperltidy command-line arguments

    Peter Eisentraut <peter@eisentraut.org> — 2023-05-25T09:10:48Z

    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.)

  2. Re: pgindent vs. pgperltidy command-line arguments

    Daniel Gustafsson <daniel@yesql.se> — 2023-05-25T09:18:28Z

    > 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 process individual 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

  3. Re: pgindent vs. pgperltidy command-line arguments

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-05-25T13:20:32Z

    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

  4. Re: pgindent vs. pgperltidy command-line arguments

    Peter Eisentraut <peter@eisentraut.org> — 2023-06-14T07:37:45Z

    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.

  5. Re: pgindent vs. pgperltidy command-line arguments

    Andrew Dunstan <andrew@dunslane.net> — 2023-06-20T15:38:10Z

    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

  6. Re: pgindent vs. pgperltidy command-line arguments

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2023-06-20T16:08:33Z

    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

  7. Re: pgindent vs. pgperltidy command-line arguments

    Peter Eisentraut <peter@eisentraut.org> — 2023-06-21T09:09:02Z

    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.

  8. Re: pgindent vs. pgperltidy command-line arguments

    Andrew Dunstan <andrew@dunslane.net> — 2023-06-21T11:35:15Z

    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

  9. Re: pgindent vs. pgperltidy command-line arguments

    Peter Eisentraut <peter@eisentraut.org> — 2023-06-21T14:36:06Z

    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

  10. Re: pgindent vs. pgperltidy command-line arguments

    Andrew Dunstan <andrew@dunslane.net> — 2023-07-06T15:47:33Z

    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