Re: WIP: SQL Formatter

On Thu, Jul 30, 2020 at 2:47 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

>
>
> 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?
>
I suggest remove node Formatting and move Formatting->SQL to Query tool
with the name "Auto-format", just like Auto complete. And yes, remove the
options which are already there in Editor.

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

--
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | *edbpostgres.com*
<http://edbpostgres.com>
"Don't Complain about Heat, Plant a TREE"