Re: WIP: SQL Formatter - Mailing list pgadmin-hackers

From Aditya Toshniwal
Subject Re: WIP: SQL Formatter
Date
Msg-id CAM9w-_k3=pwrKnpPPLbJv27V9y10CJq4+qN8hu6hdNxMDj5Niw@mail.gmail.com
Whole thread Raw
In response to Re: WIP: SQL Formatter  (Dave Page <dpage@pgadmin.org>)
Responses Re: WIP: SQL Formatter  (Dave Page <dpage@pgadmin.org>)
List pgadmin-hackers
Hi,

On Thu, Jul 30, 2020 at 1:54 PM Dave Page <dpage@pgadmin.org> wrote:


On Thu, Jul 30, 2020 at 6:29 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Dave,

On Wed, Jul 29, 2020 at 9:34 PM Dave Page <dpage@pgadmin.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
"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
"Don't Complain about Heat, Plant a TREE"

pgadmin-hackers by date:

Previous
From: Akshay Joshi
Date:
Subject: Re: [pgAdmin][SonarQube] Function returns same value
Next
From: Dave Page
Date:
Subject: Re: WIP: SQL Formatter