Re: WIP: SQL Formatter - Mailing list pgadmin-hackers
Hi Dave,
On Thu, Jul 30, 2020 at 8:22 PM Dave Page <dpage@pgadmin.org> wrote:
On Thu, Jul 30, 2020 at 10:23 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:On Thu, Jul 30, 2020 at 2:47 PM Dave Page <dpage@pgadmin.org> wrote:On Thu, Jul 30, 2020 at 9:53 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote: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.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?
Currently, all the CodeMirrors in pgAdmin use the settings from the Query tool -> Editor, even though they're not related. I think you're right. We should move all CodeMirror settings out of the Query tool and move it to a new node - "SQL" may be. Editor and Auto-format would be sub nodes under SQL. With that, the editor settings will apply at all the places where CodeMirror editing is used(function body, query tool). What do you say ?
- 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.----Thanks,Aditya ToshniwalpgAdmin hacker | Sr. Software Engineer | edbpostgres.com"Don't Complain about Heat, Plant a TREE"----Thanks,Aditya ToshniwalpgAdmin hacker | Sr. Software Engineer | edbpostgres.com"Don't Complain about Heat, Plant a TREE"----Thanks,Aditya ToshniwalpgAdmin hacker | Sr. Software Engineer | edbpostgres.com"Don't Complain about Heat, Plant a TREE"--
Thanks,
Aditya Toshniwal
pgAdmin hacker | Sr. Software Engineer | edbpostgres.com
"Don't Complain about Heat, Plant a TREE"
Attachment
pgadmin-hackers by date: