Thread: WIP: SQL Formatter

WIP: SQL Formatter

From
Dave Page
Date:
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 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.

Thanks.

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

EDB: http://www.enterprisedb.com

Attachment

Re: WIP: SQL Formatter

From
Aditya Toshniwal
Date:
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.

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

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"

Re: WIP: SQL Formatter

From
Dave Page
Date:


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.
 

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

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

Re: WIP: SQL Formatter

From
Aditya Toshniwal
Date:
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"

Re: WIP: SQL Formatter

From
Dave Page
Date:


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?

Screenshot 2020-07-30 at 10.14.31.png

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


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

EDB: http://www.enterprisedb.com

Attachment

Re: WIP: SQL Formatter

From
Aditya Toshniwal
Date:


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.

Screenshot 2020-07-30 at 10.14.31.png

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


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

Re: WIP: SQL Formatter

From
Dave Page
Date:


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?
 

Screenshot 2020-07-30 at 10.14.31.png

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


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

Attachment

Re: WIP: SQL Formatter

From
Aditya Toshniwal
Date:
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 ?
 

Screenshot 2020-07-30 at 10.14.31.png

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


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

Re: WIP: SQL Formatter

From
Dave Page
Date:
Hi

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 ?

Turns out that's more work than I really have time for at the moment, because it means making the editors be able to handle automatic reloads of multiple preference sections. We can always think about moving the formatting preferences into their own section later.

In the meantime, this update to the patch does what was originally suggested and puts the options into a Query Tool -> SQL Formatting section.

Comments? Obviously docs will need updating too, once we're all happy with the basics.

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

EDB: http://www.enterprisedb.com

Attachment

Re: WIP: SQL Formatter

From
Aditya Toshniwal
Date:
Hi,

On Fri, Jul 31, 2020 at 6:03 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

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 ?

Turns out that's more work than I really have time for at the moment, because it means making the editors be able to handle automatic reloads of multiple preference sections. We can always think about moving the formatting preferences into their own section later.

In the meantime, this update to the patch does what was originally suggested and puts the options into a Query Tool -> SQL Formatting section.
Cool. 

Comments? Obviously docs will need updating too, once we're all happy with the basics.
It works nice. A shortcut and auto format only selected text would be helpful.

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

Re: WIP: SQL Formatter

From
Dave Page
Date:


On Mon, Aug 3, 2020 at 10:31 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Fri, Jul 31, 2020 at 6:03 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

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 ?

Turns out that's more work than I really have time for at the moment, because it means making the editors be able to handle automatic reloads of multiple preference sections. We can always think about moving the formatting preferences into their own section later.

In the meantime, this update to the patch does what was originally suggested and puts the options into a Query Tool -> SQL Formatting section.
Cool. 

Comments? Obviously docs will need updating too, once we're all happy with the basics.
It works nice. A shortcut and auto format only selected text would be helpful.

Yeah, the shortcut crossed my mind. Any thoughts on what it could be? I was struggling to find something that made sense and wasn't used by Chrome or elsewhere.

Formatting only the selected text might be troublesome; it may create some very strange results if a partial statement is selected.
 
Thanks.

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

EDB: http://www.enterprisedb.com

Re: WIP: SQL Formatter

From
Aditya Toshniwal
Date:
Hi,

On Mon, Aug 3, 2020 at 3:06 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:31 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Fri, Jul 31, 2020 at 6:03 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

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 ?

Turns out that's more work than I really have time for at the moment, because it means making the editors be able to handle automatic reloads of multiple preference sections. We can always think about moving the formatting preferences into their own section later.

In the meantime, this update to the patch does what was originally suggested and puts the options into a Query Tool -> SQL Formatting section.
Cool. 

Comments? Obviously docs will need updating too, once we're all happy with the basics.
It works nice. A shortcut and auto format only selected text would be helpful.

Yeah, the shortcut crossed my mind. Any thoughts on what it could be? I was struggling to find something that made sense and wasn't used by Chrome or elsewhere.
I can only find Shift+Cmd+K available. 

Formatting only the selected text might be troublesome; it may create some very strange results if a partial statement is selected.
Formatter should not be blamed in that case. It did what was asked. :P
 
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"

Re: WIP: SQL Formatter

From
Dave Page
Date:


On Mon, Aug 3, 2020 at 10:51 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Mon, Aug 3, 2020 at 3:06 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:31 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Fri, Jul 31, 2020 at 6:03 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

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 ?

Turns out that's more work than I really have time for at the moment, because it means making the editors be able to handle automatic reloads of multiple preference sections. We can always think about moving the formatting preferences into their own section later.

In the meantime, this update to the patch does what was originally suggested and puts the options into a Query Tool -> SQL Formatting section.
Cool. 

Comments? Obviously docs will need updating too, once we're all happy with the basics.
It works nice. A shortcut and auto format only selected text would be helpful.

Yeah, the shortcut crossed my mind. Any thoughts on what it could be? I was struggling to find something that made sense and wasn't used by Chrome or elsewhere.
I can only find Shift+Cmd+K available. 

Formatting only the selected text might be troublesome; it may create some very strange results if a partial statement is selected.
Formatter should not be blamed in that case. It did what was asked. :P

*shrug*. OK. How's about this?
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com

Attachment

Re: WIP: SQL Formatter

From
Akshay Joshi
Date:
Hi Aditya

Can you please review and test.

On Fri, Aug 7, 2020 at 9:28 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:51 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Mon, Aug 3, 2020 at 3:06 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:31 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Fri, Jul 31, 2020 at 6:03 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

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 ?

Turns out that's more work than I really have time for at the moment, because it means making the editors be able to handle automatic reloads of multiple preference sections. We can always think about moving the formatting preferences into their own section later.

In the meantime, this update to the patch does what was originally suggested and puts the options into a Query Tool -> SQL Formatting section.
Cool. 

Comments? Obviously docs will need updating too, once we're all happy with the basics.
It works nice. A shortcut and auto format only selected text would be helpful.

Yeah, the shortcut crossed my mind. Any thoughts on what it could be? I was struggling to find something that made sense and wasn't used by Chrome or elsewhere.
I can only find Shift+Cmd+K available. 

Formatting only the selected text might be troublesome; it may create some very strange results if a partial statement is selected.
Formatter should not be blamed in that case. It did what was asked. :P

*shrug*. OK. How's about this?
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Re: WIP: SQL Formatter

From
Akshay Joshi
Date:
Hi Aditya

Can you please review it

On Mon, Aug 10, 2020 at 11:11 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Aditya

Can you please review and test.

On Fri, Aug 7, 2020 at 9:28 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:51 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Mon, Aug 3, 2020 at 3:06 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:31 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Fri, Jul 31, 2020 at 6:03 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

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 ?

Turns out that's more work than I really have time for at the moment, because it means making the editors be able to handle automatic reloads of multiple preference sections. We can always think about moving the formatting preferences into their own section later.

In the meantime, this update to the patch does what was originally suggested and puts the options into a Query Tool -> SQL Formatting section.
Cool. 

Comments? Obviously docs will need updating too, once we're all happy with the basics.
It works nice. A shortcut and auto format only selected text would be helpful.

Yeah, the shortcut crossed my mind. Any thoughts on what it could be? I was struggling to find something that made sense and wasn't used by Chrome or elsewhere.
I can only find Shift+Cmd+K available. 

Formatting only the selected text might be troublesome; it may create some very strange results if a partial statement is selected.
Formatter should not be blamed in that case. It did what was asked. :P

*shrug*. OK. How's about this?
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246

Re: WIP: SQL Formatter

From
Aditya Toshniwal
Date:
Hi Dave,

The patch looks good to me except pep8 issues. In the below SQL - BEGIN, RETURN, END are not formatted. But when only the BEGIN line is selected and the format button is clicked then it gets formatted. This seems to be a bug in sqlparse.
Create or replace Function public.simple_func(i_var integer) Returns integer Language 'plpgsql' cost 100 Volatile As $BODY$
BEGIN RETURN OTHER_SIMPLE_FUNC(I_VAR);
END;
$BODY$;

On Tue, Aug 18, 2020 at 10:50 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Aditya

Can you please review it

On Mon, Aug 10, 2020 at 11:11 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Aditya

Can you please review and test.

On Fri, Aug 7, 2020 at 9:28 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:51 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Mon, Aug 3, 2020 at 3:06 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:31 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Fri, Jul 31, 2020 at 6:03 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

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 ?

Turns out that's more work than I really have time for at the moment, because it means making the editors be able to handle automatic reloads of multiple preference sections. We can always think about moving the formatting preferences into their own section later.

In the meantime, this update to the patch does what was originally suggested and puts the options into a Query Tool -> SQL Formatting section.
Cool. 

Comments? Obviously docs will need updating too, once we're all happy with the basics.
It works nice. A shortcut and auto format only selected text would be helpful.

Yeah, the shortcut crossed my mind. Any thoughts on what it could be? I was struggling to find something that made sense and wasn't used by Chrome or elsewhere.
I can only find Shift+Cmd+K available. 

Formatting only the selected text might be troublesome; it may create some very strange results if a partial statement is selected.
Formatter should not be blamed in that case. It did what was asked. :P

*shrug*. OK. How's about this?
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

Re: WIP: SQL Formatter

From
Dave Page
Date:
Hi

On Tue, Aug 18, 2020 at 7:33 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Dave,

The patch looks good to me except pep8 issues.

Thanks, fixed.
 
In the below SQL - BEGIN, RETURN, END are not formatted. But when only the BEGIN line is selected and the format button is clicked then it gets formatted. This seems to be a bug in sqlparse.
Create or replace Function public.simple_func(i_var integer) Returns integer Language 'plpgsql' cost 100 Volatile As $BODY$
BEGIN RETURN OTHER_SIMPLE_FUNC(I_VAR);
END;
$BODY$;

Yeah, sqlparse isn't perfect. I suspect we'll be sending them patches or forking it... (re-indent aligned seems to be somewhat funky).

Anyway, here's a patch that includes doc updates, which I believe is complete and ready for review/commit.
 

On Tue, Aug 18, 2020 at 10:50 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Aditya

Can you please review it

On Mon, Aug 10, 2020 at 11:11 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Aditya

Can you please review and test.

On Fri, Aug 7, 2020 at 9:28 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:51 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Mon, Aug 3, 2020 at 3:06 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:31 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Fri, Jul 31, 2020 at 6:03 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

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 ?

Turns out that's more work than I really have time for at the moment, because it means making the editors be able to handle automatic reloads of multiple preference sections. We can always think about moving the formatting preferences into their own section later.

In the meantime, this update to the patch does what was originally suggested and puts the options into a Query Tool -> SQL Formatting section.
Cool. 

Comments? Obviously docs will need updating too, once we're all happy with the basics.
It works nice. A shortcut and auto format only selected text would be helpful.

Yeah, the shortcut crossed my mind. Any thoughts on what it could be? I was struggling to find something that made sense and wasn't used by Chrome or elsewhere.
I can only find Shift+Cmd+K available. 

Formatting only the selected text might be troublesome; it may create some very strange results if a partial statement is selected.
Formatter should not be blamed in that case. It did what was asked. :P

*shrug*. OK. How's about this?
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246



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

Attachment

Re: WIP: SQL Formatter

From
Akshay Joshi
Date:
Thanks, patch applied.

On Tue, Aug 18, 2020 at 3:53 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Aug 18, 2020 at 7:33 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi Dave,

The patch looks good to me except pep8 issues.

Thanks, fixed.
 
In the below SQL - BEGIN, RETURN, END are not formatted. But when only the BEGIN line is selected and the format button is clicked then it gets formatted. This seems to be a bug in sqlparse.
Create or replace Function public.simple_func(i_var integer) Returns integer Language 'plpgsql' cost 100 Volatile As $BODY$
BEGIN RETURN OTHER_SIMPLE_FUNC(I_VAR);
END;
$BODY$;

Yeah, sqlparse isn't perfect. I suspect we'll be sending them patches or forking it... (re-indent aligned seems to be somewhat funky).

Anyway, here's a patch that includes doc updates, which I believe is complete and ready for review/commit.
 

On Tue, Aug 18, 2020 at 10:50 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Aditya

Can you please review it

On Mon, Aug 10, 2020 at 11:11 AM Akshay Joshi <akshay.joshi@enterprisedb.com> wrote:
Hi Aditya

Can you please review and test.

On Fri, Aug 7, 2020 at 9:28 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:51 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Mon, Aug 3, 2020 at 3:06 PM Dave Page <dpage@pgadmin.org> wrote:


On Mon, Aug 3, 2020 at 10:31 AM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi,

On Fri, Jul 31, 2020 at 6:03 PM Dave Page <dpage@pgadmin.org> wrote:
Hi

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 ?

Turns out that's more work than I really have time for at the moment, because it means making the editors be able to handle automatic reloads of multiple preference sections. We can always think about moving the formatting preferences into their own section later.

In the meantime, this update to the patch does what was originally suggested and puts the options into a Query Tool -> SQL Formatting section.
Cool. 

Comments? Obviously docs will need updating too, once we're all happy with the basics.
It works nice. A shortcut and auto format only selected text would be helpful.

Yeah, the shortcut crossed my mind. Any thoughts on what it could be? I was struggling to find something that made sense and wasn't used by Chrome or elsewhere.
I can only find Shift+Cmd+K available. 

Formatting only the selected text might be troublesome; it may create some very strange results if a partial statement is selected.
Formatter should not be blamed in that case. It did what was asked. :P

*shrug*. OK. How's about this?
 
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EDB: http://www.enterprisedb.com



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
Thanks & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246



--
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 & Regards
Akshay Joshi
pgAdmin Hacker | Sr. Software Architect
EDB Postgres
Mobile: +91 976-788-8246