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 09:17:36
Message-ID: CA+OCxoxfaduw7zUz_1C1dTubQHDcHWURXr-Xp2SGm57p7=+Lww@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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

> Hi,
>
> On Thu, Jul 30, 2020 at 1:54 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>>
>>
>> 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.
>>
> In pgAdmin, all the SQL view(editor, sql tab) related settings are taken
> from Query tool -> Editor currently. I suggest we can add a new node -
> Autoformat parallel to Editor and put only autoformat related settings in
> there. It is understood where the other settings like tab space, etc.
> comes from. What do you think ? Currently there is no way to display groups
> in preferences, otherwise auto format would go in the editor node but in a
> group named Autoformat.
>

Well we have subsections. Currently the patch makes it look like the
following. How would you suggest we change it - remove the indent options
from the Query Tool -> Editor section and only have them in the Formatting
-> SQL section?

[image: Screenshot 2020-07-30 at 10.14.31.png]

[image: Screenshot 2020-07-30 at 10.14.50.png]

>
>>
>>>
>>>> - 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.
>>
> Yes. That can be done.
>
>>
>> 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
>>
>>
>
> --
> 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 Aditya Toshniwal 2020-07-30 09:23:20 Re: WIP: SQL Formatter
Previous Message Aditya Toshniwal 2020-07-30 08:52:34 Re: WIP: SQL Formatter