Re: WIP: SQL Formatter

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: WIP: SQL Formatter
Date: 2020-07-30 08:24:09
Message-ID: CA+OCxozhGhcgM-0r=Ov=fsMPDgHbk47M_LjuUY9b_JTUTW=Kxg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

On Thu, Jul 30, 2020 at 6:29 AM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:

> Hi Dave,
>
> On Wed, Jul 29, 2020 at 9:34 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> The attached WIP patch adds a menu option to the Query Tool to format the
>> SQL in the editor. It does so per options that can be set in the
>> Preferences panel (essentially, most of these:
>> https://sqlparse.readthedocs.io/en/latest/api/#formatting-of-sql-statements
>> )
>>
>> Some thoughts before I continue:
>>
>> - There are already options for tabs vs spaces and tab width for the
>> query tool. At the moment I've intentionally kept separate settings for the
>> same thing in the formatter. If we use the same options it'll mean that
>> configuration for formatting is split across two places in the Preferences
>> panel. On the other hand, it may be handy to have separate options. What do
>> others think?
>>
> I'm not aware of any editor who is having separate settings for
> formatting. Editors like VS code use .editorconfig for the auto format
> option. I would also suggest having common editor settings for both
> formatting and user input.
>

Fair point. Should we move the indent options into the SQL Formatting
section of the Preferences then? That seems like it could get confusing as
they'd then be separated from other settings related to formatting that are
specific to the editor.

>
>> - I'm thinking that maybe we should push all user-visible generated SQL
>> through the formatter. This would essentially mean that all get_sql and
>> similar functions would call it. We'd probably need to make the re-sql test
>> suite call it as well. Does this seem like a good idea? It's be a fairly
>> widespread change, but it would mean that the resql and generated crud
>> statements would be consistently formatted, to the user's preferences.
>>
> Yes we can. But should we use it for function/proc body ? Users may not
> like their function being altered.
>

Also a fair point. I wonder if we can just format the outer boilerplate
first, then inject the body.

Either way, that part would be a phase two project. I'm planning to get the
basic formatter in first.

>
>> Thanks.
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EDB: http://www.enterprisedb.com
>>
>>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
> <http://edbpostgres.com>
> "Don't Complain about Heat, Plant a TREE"
>

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2020-07-30 08:36:03 pgAdmin 4 commit: Fixed an issue when comparing the table with a trigge
Previous Message Aditya Toshniwal 2020-07-30 07:39:05 [pgAdmin][SonarQube] Function returns same value