Re: WIP pgindent replacement

Attached is a WIP possible replacement for pgindent. Instead of a shell
script invoking a mishmash of awk and sed, some of which is pretty
impenetrable, it uses a single engine (perl) to do all the pre and post
indent processing. Of course, if your regex-fu and perl-fu is not up the
scratch this too might be impenetrable, but all but a couple of the
recipes are reduced to single lines, and I'd argue that they are all at
least as comprehensible as what they replace.

Attached also is a diff file showing what it does differently from the
existing script. I think that these are all things where the new script
is more correct than the existing script. Most of the changes come into
two categories:

* places where the existing script fails to combine the function
return type and the function name on a single line in function
prototypes.
* places where unwanted blank lines are removed by the new script
but not by the existing script.

Features include:

* command line compatibility with the existing script, so you can do:
find ../../.. -name '*.[ch]' -type f -print | egrep -v -f
exclude_file_patterns | xargs -n100 ./pgindent.pl typedefs.list
* a new way of doing the same thing much more nicely:
./pgindent.pl --search-base=../../.. --typedefs=typedefs.list
--excludes=exclude_file_patterns
* only passes relevant typedefs to indent, not the whole huge list
* should in principle be runnable on Windows, unlike existing script
(I haven't tested yet)
* no semantic tab literals; tabs are only generated using \t and
tested for using \t, \h or \s as appropriate. This makes debugging
the script much less frustrating. If something looks like a space
it should be a space.

In one case I used perl's extended regex mode to comment a fairly hairy
regex. This should probably be done a bit more, maybe for all of them.

If anybody is so inclined, this could be used as a basis for removing
the use of bsd indent altogether, as has been suggested before, as well
as external entab/detab.

Comments welcome.

cheers

andrew

Andrew Dunstan wrote:
>
> Attached is a WIP possible replacement for pgindent. Instead of a shell
> script invoking a mishmash of awk and sed, some of which is pretty
> impenetrable, it uses a single engine (perl) to do all the pre and post
> indent processing. Of course, if your regex-fu and perl-fu is not up the
> scratch this too might be impenetrable, but all but a couple of the
> recipes are reduced to single lines, and I'd argue that they are all at
> least as comprehensible as what they replace.

I am excited Andrew has done this. It has been on my TODO list for a
while --- I was hoping someday we could switch to GNU indent but gave up
after the GNU indent report from Greg Stark that exactly matched my
experience years ago:

http://archives.postgresql.org/pgsql-hackers/2011-04/msg01436.php

Basically, GNU indent has new bugs, but bugs that are harder to work
around than the BSD indent bugs.

Once I heard that I realized we were going to be using BSD indent for
many more years to come and rewriting the shell script in Perl was the
next logical step, which Andrew has done.

This will also allow us to use pgindent on Windows that has Perl
installed --- we should create a DOS binary of bsd indent so Windows
users scan run it.

I would also like to add a command-line way to detect our patched
version of BSD indent so we know we are using the right binary. I will
do that once the Perl version is committed to git.

Thanks Andrew.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Attached is a WIP possible replacement for pgindent. Instead of a shell
> script invoking a mishmash of awk and sed, some of which is pretty
> impenetrable, it uses a single engine (perl) to do all the pre and post
> indent processing.

Hm ... this should offer the chance of running on Windows, but otherwise
I'm not sure it does very much for us, if you still have to have a
patched version of NetBSD indent underneath.

> If anybody is so inclined, this could be used as a basis for removing
> the use of bsd indent altogether, as has been suggested before, as well
> as external entab/detab.

Now *that* I would get excited about. But surely it would be a huge
amount more work?

regards, tom lane

On 06/21/2011 11:15 PM, Tom Lane wrote:
> Andrew Dunstan<andrew(at)dunslane(dot)net> writes:
>> Attached is a WIP possible replacement for pgindent. Instead of a shell
>> script invoking a mishmash of awk and sed, some of which is pretty
>> impenetrable, it uses a single engine (perl) to do all the pre and post
>> indent processing.
> Hm ... this should offer the chance of running on Windows, but otherwise
> I'm not sure it does very much for us, if you still have to have a
> patched version of NetBSD indent underneath.

Well, to start with it provides a nicer way to invoke pgindent for the
whole repo.

And it will be relatively easy to incorporate new stuff, for example
what Greg Smith was doing in his wrapper script.

It's true that the script contains an unfortunately large number of
workarounds for the limitations and failure modes of the patched bsd
indent we've been using. Some of those are currently working less than
perfectly, and I found it fairly difficult to understand exactly what
they were doing. I hope what I have produced will be a bit clearer and
more maintainable than what it replaces, as well as working a bit better
and more robustly.

>> If anybody is so inclined, this could be used as a basis for removing
>> the use of bsd indent altogether, as has been suggested before, as well
>> as external entab/detab.
> Now *that* I would get excited about. But surely it would be a huge
> amount more work?
>

Yes, which is why I decided to do this - I certainly don't currently
have time to re-implement indent.

cheers

andrew

> # Avoid bug that converts 'x =- 1' to 'x = -1'

> $source =~ s!=- !-= !g;

I haven't looked at the shell script this replaces, but is that the correct substitution pattern? (BTW, I'm not seeing the token =- anywhere except in the Makefile, which wouldn't be run against, no? Am I missing something?)

Regards,

David
--
David Christensen
End Point Corporation
david(at)endpoint(dot)com

On 06/22/2011 02:03 AM, David Christensen wrote:
>> # Avoid bug that converts 'x =- 1' to 'x = -1'
>> $source =~ s!=- !-= !g;
>
> I haven't looked at the shell script this replaces, but is that the correct substitution pattern? (BTW, I'm not seeing the token =- anywhere except in the Makefile, which wouldn't be run against, no? Am I missing something?)
>
>

It's exactly what the current script does. The reason you don't see this
anywhere is that previous pgindent runs have removed it. We don't undo
the transformation. But maybe we should just get rid of it.

cheers

andrew

On 06/22/2011 08:35 AM, Andrew Dunstan wrote:
>
>
> On 06/22/2011 02:03 AM, David Christensen wrote:
>>> # Avoid bug that converts 'x =- 1' to 'x = -1'
>>> $source =~ s!=- !-= !g;
>>
>> I haven't looked at the shell script this replaces, but is that the
>> correct substitution pattern? (BTW, I'm not seeing the token =-
>> anywhere except in the Makefile, which wouldn't be run against, no?
>> Am I missing something?)
>>
>>
>
>
> It's exactly what the current script does. The reason you don't see
> this anywhere is that previous pgindent runs have removed it. We don't
> undo the transformation. But maybe we should just get rid of it.
>
>

Further research shows that C89 explicitly dropped support for the old
K&R "=-" operator, so we probably *should* remove this in case it
introduces an unintended bug.

cheers

andrew

Andrew Dunstan wrote:
>
>
> On 06/22/2011 08:35 AM, Andrew Dunstan wrote:
> >
> >
> > On 06/22/2011 02:03 AM, David Christensen wrote:
> >>> # Avoid bug that converts 'x =- 1' to 'x = -1'
> >>> $source =~ s!=- !-= !g;
> >>
> >> I haven't looked at the shell script this replaces, but is that the
> >> correct substitution pattern? (BTW, I'm not seeing the token =-
> >> anywhere except in the Makefile, which wouldn't be run against, no?
> >> Am I missing something?)
> >>
> >>
> >
> >
> > It's exactly what the current script does. The reason you don't see
> > this anywhere is that previous pgindent runs have removed it. We don't
> > undo the transformation. But maybe we should just get rid of it.
> >
> >
>
> Further research shows that C89 explicitly dropped support for the old
> K&R "=-" operator, so we probably *should* remove this in case it
> introduces an unintended bug.

Well, the point is if someone does use that, it isn't going to generate
a pgindent error, but rather produce incorrect C code because =- is
going to be changed. FYI, my gcc 2.95.3 allows =- and does work as
intended.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 06/22/2011 09:08 AM, Bruce Momjian wrote:
> Andrew Dunstan wrote:
>>
>> On 06/22/2011 08:35 AM, Andrew Dunstan wrote:
>>>
>>> On 06/22/2011 02:03 AM, David Christensen wrote:
>>>>> # Avoid bug that converts 'x =- 1' to 'x = -1'
>>>>> $source =~ s!=- !-= !g;
>>>> I haven't looked at the shell script this replaces, but is that the
>>>> correct substitution pattern? (BTW, I'm not seeing the token =-
>>>> anywhere except in the Makefile, which wouldn't be run against, no?
>>>> Am I missing something?)
>>>>
>>>>
>>>
>>> It's exactly what the current script does. The reason you don't see
>>> this anywhere is that previous pgindent runs have removed it. We don't
>>> undo the transformation. But maybe we should just get rid of it.
>>>
>>>
>> Further research shows that C89 explicitly dropped support for the old
>> K&R "=-" operator, so we probably *should* remove this in case it
>> introduces an unintended bug.
> Well, the point is if someone does use that, it isn't going to generate
> a pgindent error, but rather produce incorrect C code because =- is
> going to be changed. FYI, my gcc 2.95.3 allows =- and does work as
> intended.
>

As intended by whom? If the effect of "x=4; x =- 1;" is to subtract 1
from x then that's simply wrong by C89. It should assign -1 to x. The
"=-" must be parsed as two operators in C89, assignment and unary minus.
pgindent should not under any circumstances change the semantics of the
program being indented, and that's what this transformation does for
compilers conforming to the standard we explicitly follow.

What happens when your ancient gcc is told to apply the ansi standard?

cheers

andrew

Andrew Dunstan wrote:
> >> Further research shows that C89 explicitly dropped support for the old
> >> K&R "=-" operator, so we probably *should* remove this in case it
> >> introduces an unintended bug.
> > Well, the point is if someone does use that, it isn't going to generate
> > a pgindent error, but rather produce incorrect C code because =- is
> > going to be changed. FYI, my gcc 2.95.3 allows =- and does work as
> > intended.
> >
>
> As intended by whom? If the effect of "x=4; x =- 1;" is to subtract 1
> from x then that's simply wrong by C89. It should assign -1 to x. The
> "=-" must be parsed as two operators in C89, assignment and unary minus.
> pgindent should not under any circumstances change the semantics of the
> program being indented, and that's what this transformation does for
> compilers conforming to the standard we explicitly follow.
>
> What happens when your ancient gcc is told to apply the ansi standard?

I see now that my test wasn't complete. You are right it assigns -1 so
we can remove this from pgupgrade.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Wed, Jun 22, 2011 at 10:16:28AM -0400, Bruce Momjian wrote:
> Andrew Dunstan wrote:
> > >> Further research shows that C89 explicitly dropped support for the old
> > >> K&R "=-" operator, so we probably *should* remove this in case it
> > >> introduces an unintended bug.
> > > Well, the point is if someone does use that, it isn't going to generate
> > > a pgindent error, but rather produce incorrect C code because =- is
> > > going to be changed. FYI, my gcc 2.95.3 allows =- and does work as
> > > intended.
> > >
> >
> > As intended by whom? If the effect of "x=4; x =- 1;" is to subtract 1
> > from x then that's simply wrong by C89. It should assign -1 to x. The
> > "=-" must be parsed as two operators in C89, assignment and unary minus.
> > pgindent should not under any circumstances change the semantics of the
> > program being indented, and that's what this transformation does for
> > compilers conforming to the standard we explicitly follow.
> >
> > What happens when your ancient gcc is told to apply the ansi standard?
>
> I see now that my test wasn't complete. You are right it assigns -1 so
> we can remove this from pgindent.

Per report form last year, removed from pgindent.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Tue, Jun 21, 2011 at 08:27:45PM -0400, Andrew Dunstan wrote:
>
> Attached is a WIP possible replacement for pgindent. Instead of a
> shell script invoking a mishmash of awk and sed, some of which is
> pretty impenetrable, it uses a single engine (perl) to do all the
> pre and post indent processing. Of course, if your regex-fu and
> perl-fu is not up the scratch this too might be impenetrable, but
> all but a couple of the recipes are reduced to single lines, and I'd
> argue that they are all at least as comprehensible as what they
> replace.
>
> Attached also is a diff file showing what it does differently from
> the existing script. I think that these are all things where the new
> script is more correct than the existing script. Most of the changes
> come into two categories:
>
> * places where the existing script fails to combine the function
> return type and the function name on a single line in function
> prototypes.
> * places where unwanted blank lines are removed by the new script
> but not by the existing script.
>
> Features include:
>
> * command line compatibility with the existing script, so you can do:
> find ../../.. -name '*.[ch]' -type f -print | egrep -v -f
> exclude_file_patterns | xargs -n100 ./pgindent.pl typedefs.list
> * a new way of doing the same thing much more nicely:
> ./pgindent.pl --search-base=../../.. --typedefs=typedefs.list
> --excludes=exclude_file_patterns
> * only passes relevant typedefs to indent, not the whole huge list
> * should in principle be runnable on Windows, unlike existing script
> (I haven't tested yet)
> * no semantic tab literals; tabs are only generated using \t and
> tested for using \t, \h or \s as appropriate. This makes debugging
> the script much less frustrating. If something looks like a space
> it should be a space.
>
> In one case I used perl's extended regex mode to comment a fairly
> hairy regex. This should probably be done a bit more, maybe for all
> of them.
>
> If anybody is so inclined, this could be used as a basis for
> removing the use of bsd indent altogether, as has been suggested
> before, as well as external entab/detab.

Thirteen months after Andrew posted this WIP, I have restructured and
tested this code, and it is now ready to replace the pgindent shell
script as pgindent.pl, attached.

I have tested this version by re-running the 9.1 and 9.2 pgindent runs
and comparing the output, and it is just like Andrew said --- it is the
same, except for the two improvements he mentioned.

A Perl version of pgindent has several advantages:

* more portable; less dependent on utility command variances
* able to run on Windows, assuming someone makes entab and
pg_bsd_indent Windows binaries
* able to fix more limitations of pgindent

I will add documentation about the arguments.

Many thanks to Andrew for his fine work on this. Any objections?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 08/02/2012 11:09 PM, Bruce Momjian wrote:

> Thirteen months after Andrew posted this WIP, I have restructured and
> tested this code, and it is now ready to replace the pgindent shell
> script as pgindent.pl, attached.
>
> I have tested this version by re-running the 9.1 and 9.2 pgindent runs
> and comparing the output, and it is just like Andrew said --- it is the
> same, except for the two improvements he mentioned.
>
> A Perl version of pgindent has several advantages:
>
> * more portable; less dependent on utility command variances
> * able to run on Windows, assuming someone makes entab and
> pg_bsd_indent Windows binaries
> * able to fix more limitations of pgindent
>
> I will add documentation about the arguments.
>
> Many thanks to Andrew for his fine work on this. Any objections?

Thanks for not forgetting this.

I think we generally don't put file type extensions on commands, so this
should probably just be renamed pgindent. If someone wants to go back to
the old shell script they can still get it from git.

cheers

andrew

On Fri, Aug 3, 2012 at 08:26:50AM -0400, Andrew Dunstan wrote:
>
> On 08/02/2012 11:09 PM, Bruce Momjian wrote:
>
> >Thirteen months after Andrew posted this WIP, I have restructured and
> >tested this code, and it is now ready to replace the pgindent shell
> >script as pgindent.pl, attached.
> >
> >I have tested this version by re-running the 9.1 and 9.2 pgindent runs
> >and comparing the output, and it is just like Andrew said --- it is the
> >same, except for the two improvements he mentioned.
> >
> >A Perl version of pgindent has several advantages:
> >
> >* more portable; less dependent on utility command variances
> >* able to run on Windows, assuming someone makes entab and
> > pg_bsd_indent Windows binaries
> >* able to fix more limitations of pgindent
> >
> >I will add documentation about the arguments.
> >
> >Many thanks to Andrew for his fine work on this. Any objections?
>
>
> Thanks for not forgetting this.
>
> I think we generally don't put file type extensions on commands, so
> this should probably just be renamed pgindent. If someone wants to
> go back to the old shell script they can still get it from git.

Of course. I was just noticing that most of the Perl scripts in
/src/tools and src/tools/msvc have a .pl extension on the file name, so
I was following that style. Is that valid?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> On Fri, Aug 3, 2012 at 08:26:50AM -0400, Andrew Dunstan wrote:
>> I think we generally don't put file type extensions on commands, so
>> this should probably just be renamed pgindent. If someone wants to
>> go back to the old shell script they can still get it from git.

> Of course. I was just noticing that most of the Perl scripts in
> /src/tools and src/tools/msvc have a .pl extension on the file name, so
> I was following that style. Is that valid?

Well, you're replacing the old script, so it has to keep the same name.

IMO adding such an extension to an executable script isn't a terribly
good practice, because it turns what ought to be an implementation
detail into part of the script's API. Had the shell script been named
pgindent.sh to begin with, we'd now be stuck with the unpalatable
alternatives of changing the name or using an extension that lies
about the implementation language. I don't much care for putting
in an assumption that the Perl implementation will never be replaced,
either.

regards, tom lane

On Fri, Aug 3, 2012 at 10:33 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> On Fri, Aug 3, 2012 at 08:26:50AM -0400, Andrew Dunstan wrote:
>>> I think we generally don't put file type extensions on commands, so
>>> this should probably just be renamed pgindent. If someone wants to
>>> go back to the old shell script they can still get it from git.
>
>> Of course. I was just noticing that most of the Perl scripts in
>> /src/tools and src/tools/msvc have a .pl extension on the file name, so
>> I was following that style. Is that valid?
>
> Well, you're replacing the old script, so it has to keep the same name.
>
> IMO adding such an extension to an executable script isn't a terribly
> good practice, because it turns what ought to be an implementation
> detail into part of the script's API. Had the shell script been named
> pgindent.sh to begin with, we'd now be stuck with the unpalatable
> alternatives of changing the name or using an extension that lies
> about the implementation language. I don't much care for putting
> in an assumption that the Perl implementation will never be replaced,
> either.

+1.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Aug 3, 2012 at 10:33:21AM -0400, Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > On Fri, Aug 3, 2012 at 08:26:50AM -0400, Andrew Dunstan wrote:
> >> I think we generally don't put file type extensions on commands, so
> >> this should probably just be renamed pgindent. If someone wants to
> >> go back to the old shell script they can still get it from git.
>
> > Of course. I was just noticing that most of the Perl scripts in
> > /src/tools and src/tools/msvc have a .pl extension on the file name, so
> > I was following that style. Is that valid?
>
> Well, you're replacing the old script, so it has to keep the same name.
>
> IMO adding such an extension to an executable script isn't a terribly
> good practice, because it turns what ought to be an implementation
> detail into part of the script's API. Had the shell script been named
> pgindent.sh to begin with, we'd now be stuck with the unpalatable
> alternatives of changing the name or using an extension that lies
> about the implementation language. I don't much care for putting
> in an assumption that the Perl implementation will never be replaced,
> either.

OK, sure, we can keep the pgindent name --- I was just trying to be
consistent. One problem with the lack of an extension is that there is
no easy way for the Perl cleanup instructions to find all the Perl
executables --- right now it looks for an extension. Do we have other
Perl scripts in our tree that don't end in *.pl or *.pm? I didn't find
any with this script:

$ find . -type f -exec file {} \;|grep Perl
./src/backend/catalog/Catalog.pm: Perl5 module source text
./src/tools/msvc/MSBuildProject.pm: Perl5 module source text
./src/tools/msvc/Project.pm: Perl5 module source text
./src/tools/msvc/Mkvcbuild.pm: Perl5 module source text
./src/tools/msvc/Install.pm: Perl5 module source text
./src/tools/msvc/Solution.pm: Perl5 module source text
./src/tools/msvc/VCBuildProject.pm: Perl5 module source text
./src/tools/msvc/VSObjectFactory.pm: Perl5 module source text

We can hard-code pgindent as one we chould perltidy.

FYI, personally, I have never been a big fan of using *.pl for things I
normally run, like scripts in /usr/local/bin, but I sometimes use *.pl
for utility stuff.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 08/03/2012 10:51 AM, Bruce Momjian wrote:

> OK, sure, we can keep the pgindent name --- I was just trying to be
> consistent. One problem with the lack of an extension is that there is
> no easy way for the Perl cleanup instructions to find all the Perl
> executables --- right now it looks for an extension. Do we have other
> Perl scripts in our tree that don't end in *.pl or *.pm? I didn't find
> any with this script:
>
> $ find . -type f -exec file {} \;|grep Perl
> ./src/backend/catalog/Catalog.pm: Perl5 module source text
> ./src/tools/msvc/MSBuildProject.pm: Perl5 module source text
> ./src/tools/msvc/Project.pm: Perl5 module source text
> ./src/tools/msvc/Mkvcbuild.pm: Perl5 module source text
> ./src/tools/msvc/Install.pm: Perl5 module source text
> ./src/tools/msvc/Solution.pm: Perl5 module source text
> ./src/tools/msvc/VCBuildProject.pm: Perl5 module source text
> ./src/tools/msvc/VSObjectFactory.pm: Perl5 module source text

Try:

find . -exec file {} \; | egrep 'perl script|perl -w script|Perl5
module'

cheers

andrew

On Fri, Aug 3, 2012 at 12:51:03PM -0400, Andrew Dunstan wrote:
>
> On 08/03/2012 10:51 AM, Bruce Momjian wrote:
>
>
> >OK, sure, we can keep the pgindent name --- I was just trying to be
> >consistent. One problem with the lack of an extension is that there is
> >no easy way for the Perl cleanup instructions to find all the Perl
> >executables --- right now it looks for an extension. Do we have other
> >Perl scripts in our tree that don't end in *.pl or *.pm? I didn't find
> >any with this script:
> >
> > $ find . -type f -exec file {} \;|grep Perl
> > ./src/backend/catalog/Catalog.pm: Perl5 module source text
> > ./src/tools/msvc/MSBuildProject.pm: Perl5 module source text
> > ./src/tools/msvc/Project.pm: Perl5 module source text
> > ./src/tools/msvc/Mkvcbuild.pm: Perl5 module source text
> > ./src/tools/msvc/Install.pm: Perl5 module source text
> > ./src/tools/msvc/Solution.pm: Perl5 module source text
> > ./src/tools/msvc/VCBuildProject.pm: Perl5 module source text
> > ./src/tools/msvc/VSObjectFactory.pm: Perl5 module source text
>
> Try:
>
> find . -exec file {} \; | egrep 'perl script|perl -w script|Perl5
> module'

OK, I used:

$ find . -type f -exec file {} \;|
egrep -i 'perl.*(script|module)'|grep -v '\.p[lm]:'

and got:

./src/pl/plperl/ppport.h: awk script text
./src/tools/pginclude/pgcheckdefines: a /usr/bin/perl -w script text executable
./src/tools/git_changelog: a /usr/bin/perl script text executable

The last two are Perl scripts without Perl file extensions, so let's
just go with 'pgindent' and I will hard-code those into the perltidy
instructions.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On 08/03/2012 01:23 PM, Bruce Momjian wrote:
> On Fri, Aug 3, 2012 at 12:51:03PM -0400, Andrew Dunstan wrote:
>> On 08/03/2012 10:51 AM, Bruce Momjian wrote:
>>
>>
>>> OK, sure, we can keep the pgindent name --- I was just trying to be
>>> consistent. One problem with the lack of an extension is that there is
>>> no easy way for the Perl cleanup instructions to find all the Perl
>>> executables --- right now it looks for an extension. Do we have other
>>> Perl scripts in our tree that don't end in *.pl or *.pm? I didn't find
>>> any with this script:
>>>
>>> $ find . -type f -exec file {} \;|grep Perl
>>> ./src/backend/catalog/Catalog.pm: Perl5 module source text
>>> ./src/tools/msvc/MSBuildProject.pm: Perl5 module source text
>>> ./src/tools/msvc/Project.pm: Perl5 module source text
>>> ./src/tools/msvc/Mkvcbuild.pm: Perl5 module source text
>>> ./src/tools/msvc/Install.pm: Perl5 module source text
>>> ./src/tools/msvc/Solution.pm: Perl5 module source text
>>> ./src/tools/msvc/VCBuildProject.pm: Perl5 module source text
>>> ./src/tools/msvc/VSObjectFactory.pm: Perl5 module source text
>> Try:
>>
>> find . -exec file {} \; | egrep 'perl script|perl -w script|Perl5
>> module'
> OK, I used:
>
> $ find . -type f -exec file {} \;|
> egrep -i 'perl.*(script|module)'|grep -v '\.p[lm]:'
>
> and got:
>
> ./src/pl/plperl/ppport.h: awk script text
> ./src/tools/pginclude/pgcheckdefines: a /usr/bin/perl -w script text executable
> ./src/tools/git_changelog: a /usr/bin/perl script text executable
>
> The last two are Perl scripts without Perl file extensions, so let's
> just go with 'pgindent' and I will hard-code those into the perltidy
> instructions.
>

Your pattern has produced a false positive, though. Wouldn't it be
better not to hardcode anything?

cheers

andrew

On Fri, Aug 3, 2012 at 02:08:34PM -0400, Andrew Dunstan wrote:
> >$ find . -type f -exec file {} \;|
> > egrep -i 'perl.*(script|module)'|grep -v '\.p[lm]:'
> >
> >and got:
> >
> >./src/pl/plperl/ppport.h: awk script text
> >./src/tools/pginclude/pgcheckdefines: a /usr/bin/perl -w script text executable
> >./src/tools/git_changelog: a /usr/bin/perl script text executable
> >
> >The last two are Perl scripts without Perl file extensions, so let's
> >just go with 'pgindent' and I will hard-code those into the perltidy
> >instructions.
> >
>
>
>
> Your pattern has produced a false positive, though. Wouldn't it be
> better not to hardcode anything?

You mean use an actual 'grep' to find the Perl programs --- I can do
that.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

On Fri, Aug 3, 2012 at 02:53:08PM -0400, Bruce Momjian wrote:
> On Fri, Aug 3, 2012 at 02:08:34PM -0400, Andrew Dunstan wrote:
> > >$ find . -type f -exec file {} \;|
> > > egrep -i 'perl.*(script|module)'|grep -v '\.p[lm]:'
> > >
> > >and got:
> > >
> > >./src/pl/plperl/ppport.h: awk script text
> > >./src/tools/pginclude/pgcheckdefines: a /usr/bin/perl -w script text executable
> > >./src/tools/git_changelog: a /usr/bin/perl script text executable
> > >
> > >The last two are Perl scripts without Perl file extensions, so let's
> > >just go with 'pgindent' and I will hard-code those into the perltidy
> > >instructions.
> > >
> >
> >
> >
> > Your pattern has produced a false positive, though. Wouldn't it be
> > better not to hardcode anything?
>
> You mean use an actual 'grep' to find the Perl programs --- I can do
> that.

OK, pgindent replaced by Perl version. I had to go with both a file
extension check and 'file' output check because some Perl scripts only
match one category; checks are:

(
find . -name \*.pl -o -name \*.pm

find . -type f -exec file {} \; |
egrep -i ':.*perl[0-9]*\>' |
cut -d: -f1
) |
sort -u |
xargs perltidy --profile=src/tools/pgindent/perltidyrc

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +