Re: WIP: SQL Formatter

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

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

My concern with that (which I've always had with the other formatting
options that affect CodeMirror directly) is that they are not related
*only* to the Query Tool; in this case, in the future it may affect resql
as well.

Maybe the answer is to go the other way, and move everything under
Formatting?

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

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

EDB: http://www.enterprisedb.com