Re: Review: psql include file using relative path

On Mon, Jun 6, 2011 at 10:11 PM, Gurjeet Singh <singh.gurjeet@gmail.com> wrote:
> On Mon, Jun 6, 2011 at 9:48 PM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
>>
>> On Sun, Jun 5, 2011 at 8:16 PM, Gurjeet Singh <singh.gurjeet@gmail.com>
>> wrote:
>> > Attached an updated patch.
>> >
>> > If you find it ready for committer, please mark it so in the commitfest
>> > app.
>>
>> I can't find anything further to nitpick with this patch, and have
>> marked it Ready For Committer in the CF. Thanks for your work on this,
>> I am looking forward to the feature.
>
> Thanks for your reviews and perseverance :)

I committed this after substantial further revisions:

- I rewrote the changes to process_file() to use the pathname-handling
functions in src/port, rather custom code.  Along the way, relpath
became a constant-size buffer, which should be OK since
join_pathname_components() knows about MAXPGPATH.  This has what I
consider to be a useful side effect of not calling pg_malloc() here,
which means we don't have to remember to free the memory.

- I added a safeguard against someone doing something like "\ir E:foo"
on Windows.  Although that's not an absolute path, for purposes of \ir
it needs to be treated as one.  I don't have a Windows build
environment handy so someone may want to test that I haven't muffed
this.

- I rewrote the documentation and a number of the comments to be (I
hope) more clear.

- I reverted some unnecessary whitespace changes in exec_command().

- As proposed, the patch declared process_file with a non-constant
initialized and then declared another variable after that.  I believe
some old compilers will barf on that.  Since it isn't needed in that
block anyway, I moved it to an inner block.

- I incremented the pager line count for psql's help.

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