Patch to make pgindent work cleanly

Lists: pgsql-hackers
From: Gurjeet Singh <gurjeet(at)singh(dot)im>
To: PGSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Patch to make pgindent work cleanly
Date: 2013-02-19 21:50:45
Message-ID: CABwTF4WJG_QWaGruphXdVbOgysO5br9ad5pNk2bp1eFswHHkbw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Please find attached the patch for some cleanup and fix bit rot in pgindent
script.

There were a few problems with the script.

.) It failed to use the $ENV{PGENTAB} even if it was set.
.) The file it tries to download from Postgres' ftp site is no longer
present.
ftp://ftp.postgresql.org/pub/dev/indent.netbsd.patched.tgz
.) The tar command extracted the above-mentioned file to a child directory,
but did not descend into it before running make on it.
I think it expected a tarbomb, but clearly the current .tgz file on ftp
site is not a tarbomb.

I don't like the fact that it dies with a message "fetching xyz" rather
than saying "Could not fetch xyz", but this patch does not address that
since it doesn't really affect the output when script does work.

With this patch in place I could very easily run it on any arbitrary file,
which seemed like a black-magic before the patch.

src/tools/pgindent/pgindent --build <your file path here>

--
Gurjeet Singh

http://gurjeet.singh.im/

EnterprsieDB Inc.

Attachment Content-Type Size
pgindent.patch application/octet-stream 1.2 KB

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Gurjeet Singh <gurjeet(at)singh(dot)im>
Cc: PGSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch to make pgindent work cleanly
Date: 2013-04-12 15:44:58
Message-ID: 20130412154458.GA28226@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Feb 19, 2013 at 04:50:45PM -0500, Gurjeet Singh wrote:
> Please find attached the patch for some cleanup and fix bit rot in pgindent
> script.
>
> There were a few problems with the script.
>
> .) It failed to use the $ENV{PGENTAB} even if it was set.
> .) The file it tries to download from Postgres' ftp site is no longer present.
> ftp://ftp.postgresql.org/pub/dev/indent.netbsd.patched.tgz
> .) The tar command extracted the above-mentioned file to a child directory, but
> did not descend into it before running make on it.
> I think it expected a tarbomb, but clearly the current .tgz file on ftp
> site is not a tarbomb.
>
> I don't like the fact that it dies with a message "fetching xyz" rather than
> saying "Could not fetch xyz", but this patch does not address that since it
> doesn't really affect the output when script does work.
>
> With this patch in place I could very easily run it on any arbitrary file,
> which seemed like a black-magic before the patch.
>
> src/tools/pgindent/pgindent --build <your file path here>

I have applied this patch. However, I modified the tarball name to
reference $INDENT_VERSION, rather than hard-coding "1.2".

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

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


From: Gurjeet Singh <gurjeet(at)singh(dot)im>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: PGSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch to make pgindent work cleanly
Date: 2013-04-12 17:34:49
Message-ID: CABwTF4WTHFLr7=3d8k06F8hcEoNUKxEsdLyb1cSOtPKQOaBDjw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Apr 12, 2013 at 11:44 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> On Tue, Feb 19, 2013 at 04:50:45PM -0500, Gurjeet Singh wrote:
> > Please find attached the patch for some cleanup and fix bit rot in
> pgindent
> > script.
> >
> > There were a few problems with the script.
> >
> > .) It failed to use the $ENV{PGENTAB} even if it was set.
> > .) The file it tries to download from Postgres' ftp site is no longer
> present.
> > ftp://ftp.postgresql.org/pub/dev/indent.netbsd.patched.tgz
> > .) The tar command extracted the above-mentioned file to a child
> directory, but
> > did not descend into it before running make on it.
> > I think it expected a tarbomb, but clearly the current .tgz file on
> ftp
> > site is not a tarbomb.
> >
> > I don't like the fact that it dies with a message "fetching xyz" rather
> than
> > saying "Could not fetch xyz", but this patch does not address that since
> it
> > doesn't really affect the output when script does work.
> >
> > With this patch in place I could very easily run it on any arbitrary
> file,
> > which seemed like a black-magic before the patch.
> >
> > src/tools/pgindent/pgindent --build <your file path here>
>
> I have applied this patch. However, I modified the tarball name to
> reference $INDENT_VERSION, rather than hard-coding "1.2".

Thanks!

Can you also improve the output when it dies upon failure to fetch
something? Currently the only error message it emits is "fetching xyz", and
leaves the user confused as to what really the problem was. The only
indication of a problem might be the exit code, but I'm not sure of that,
and that doesn't help the interactive user running it on terminal.

--
Gurjeet Singh

http://gurjeet.singh.im/

EnterpriseDB Inc.


From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Gurjeet Singh <gurjeet(at)singh(dot)im>
Cc: PGSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch to make pgindent work cleanly
Date: 2013-04-12 19:26:47
Message-ID: 20130412192647.GE28226@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Apr 12, 2013 at 01:34:49PM -0400, Gurjeet Singh wrote:
> Can you also improve the output when it dies upon failure to fetch something?
> Currently the only error message it emits is "fetching xyz", and leaves the
> user confused as to what really the problem was. The only indication of a
> problem might be the exit code, but I'm not sure of that, and that doesn't
> help the interactive user running it on terminal.

Good point. I have reviewed all the error messages and improved them
with the attached, applied patch, e.g.:

cannot locate typedefs file "xyz" at /usr/local/bin/pgindent line 121.

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

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

Attachment Content-Type Size
pgindent.diff text/x-diff 3.2 KB

From: Gurjeet Singh <gurjeet(at)singh(dot)im>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: PGSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch to make pgindent work cleanly
Date: 2013-04-12 19:33:00
Message-ID: CABwTF4VrhXigQz7AL4-qVnLYF=iuL6mnC=N6PFg7xSxM3cLbWQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Apr 12, 2013 at 3:26 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> On Fri, Apr 12, 2013 at 01:34:49PM -0400, Gurjeet Singh wrote:
> > Can you also improve the output when it dies upon failure to fetch
> something?
> > Currently the only error message it emits is "fetching xyz", and leaves
> the
> > user confused as to what really the problem was. The only indication of a
> > problem might be the exit code, but I'm not sure of that, and that
> doesn't
> > help the interactive user running it on terminal.
>
> Good point. I have reviewed all the error messages and improved them
> with the attached, applied patch, e.g.:
>
> cannot locate typedefs file "xyz" at /usr/local/bin/pgindent line
> 121.
>

Thanks!

--
Gurjeet Singh

http://gurjeet.singh.im/

EnterpriseDB Inc.