Re: WIP: SQL Formatter

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>
Cc: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: WIP: SQL Formatter
Date: 2020-08-18 10:23:32
Message-ID: CA+OCxowxGHXNO1rJYD_BFC-CMN=dTgswYBsnTgyua950_89euA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi

On Tue, Aug 18, 2020 at 7:33 AM Aditya Toshniwal <
aditya(dot)toshniwal(at)enterprisedb(dot)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(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Aditya
>>
>> Can you please review it
>>
>> On Mon, Aug 10, 2020 at 11:11 AM Akshay Joshi <
>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Aditya
>>>
>>> Can you please review and test.
>>>
>>> On Fri, Aug 7, 2020 at 9:28 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>
>>>>
>>>>
>>>> On Mon, Aug 3, 2020 at 10:51 AM Aditya Toshniwal <
>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> On Mon, Aug 3, 2020 at 3:06 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Aug 3, 2020 at 10:31 AM Aditya Toshniwal <
>>>>>> aditya(dot)toshniwal(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Fri, Jul 31, 2020 at 6:03 PM Dave Page <dpage(at)pgadmin(dot)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 <http://edbpostgres.com>*
>>>
>>> *Mobile: +91 976-788-8246*
>>>
>>
>>
>> --
>> *Thanks & Regards*
>> *Akshay Joshi*
>> *pgAdmin Hacker | Sr. Software Architect*
>> *EDB Postgres <http://edbpostgres.com>*
>>
>> *Mobile: +91 976-788-8246*
>>
>
>
> --
> 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

Attachment Content-Type Size
SQL_Formatting_v4.patch application/octet-stream 901.0 KB

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2020-08-18 12:29:47 pgAdmin 4 commit: Ensure that 'bcrypt' version should be less than or e
Previous Message Dave Page 2020-08-18 10:15:27 pgAdmin 4 commit: Fix typo