Re: WIP: SQL Formatter

From: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: pgadmin-hackers <pgadmin-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: WIP: SQL Formatter
Date: 2020-07-30 08:52:34
Message-ID: CAM9w-_k3=pwrKnpPPLbJv27V9y10CJq4+qN8hu6hdNxMDj5Niw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

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.

>
>
>>
>>> - 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"

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Dave Page 2020-07-30 09:17:36 Re: WIP: SQL Formatter
Previous Message Akshay Joshi 2020-07-30 08:36:36 Re: [pgAdmin][SonarQube] Function returns same value