Re: SQL Code Formatting Patch

From: "Edward Di Geronimo Jr(dot)" <edigeronimo(at)xtracards(dot)com>
To: Dave Page <dpage(at)vale-housing(dot)co(dot)uk>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: SQL Code Formatting Patch
Date: 2006-06-13 16:05:34
Message-ID: 20060613120534.imwcyxc0j9ws8kgw@webmail.picoip.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Quoting Dave Page <dpage(at)vale-housing(dot)co(dot)uk>:

> I would expect the subselects to look more like:
>
> (
> SELECT ((((h2.hd_hardware_id::text || ' ('::character
> varying::text) || a2.gbl_name::text) || ',
> '::character varying::text) ||
> h2.hd_model::text) || ')'::character varying::text
> FROM hd_hardware h2,
> gbl_addr_book a2
> WHERE h2.hd_manufacturer = a2.gbl_guid
> AND h2.hd_guid = sr_header.sr_hardware_id
> ) AS hardware,
>
> Having the braces in non-matched positions decreases the readability I
> think. Thoughts?

I agree that it would be better as you wrote, and that was my original
intention, but I'm having trouble pulling that off without side
effects that make things look much worse. I've got a version that
handles the parens around the select ok, but, has issues with the
other parens in that example. The above would come out something like
this:

(
SELECT ((((h2.hd_hardware_id::text || ' ('::character
varying::text
) || a2.gbl_name::text
) || ', '::character varying::text
) || h2.hd_model::text
) || ')'::character varying::text
FROM hd_hardware h2,
gbl_addr_book a2
WHERE h2.hd_manufacturer = a2.gbl_guid
AND h2.hd_guid = sr_header.sr_hardware_id
) AS hardware,

I'll look at it a little more, but I don't have high hopes of getting
it better without making the scanner try to understand the SQL instead
of the current approach of just reacting to things of interest. I do
have an idea that may work, but I'm not sure yet. It'll take a while
for me to work out, and I'm not sure how quickly I'll be able to get
to it, so you may want to go ahead with the current patch for now.

BTW, there's a table of keywords in the code that defines what should
be done when they're detected. There's a bool at the end that
determines if a new line should be inserted before the keyword. You
may want to change that to false for the CASE keyword. I wasn't sure
if there were instances where that would backfire, so I figured extra
newlines were better than not enough of them. My main concern is if
you try using case somewhere in the where clause.

Ed

----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program.

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2006-06-13 20:00:08 Re: SQL Code Formatting Patch
Previous Message svn 2006-06-13 14:46:07 SVN Commit by dpage: r5226 - in trunk/pgadmin3: . src/schema