Re: WIP: SQL Formatter

From: Akshay Joshi <akshay(dot)joshi(at)enterprisedb(dot)com>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Aditya Toshniwal <aditya(dot)toshniwal(at)enterprisedb(dot)com>, pgadmin-hackers <pgadmin-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: WIP: SQL Formatter
Date: 2020-08-20 08:38:26
Message-ID: CANxoLDfJbUAMin9qB_y7g9bYtcUOjti+x8VJR67GSLN=dqG=bA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Thanks, patch applied.

On Tue, Aug 18, 2020 at 3:53 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:

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

--
*Thanks & Regards*
*Akshay Joshi*
*pgAdmin Hacker | Sr. Software Architect*
*EDB Postgres <http://edbpostgres.com>*

*Mobile: +91 976-788-8246*

In response to

Browse pgadmin-hackers by date

  From Date Subject
Next Message Akshay Joshi 2020-08-20 08:39:01 Re: [pgAdmin][RM5344][Code Coverage] Improve API test cases for Grant Wizard
Previous Message Akshay Joshi 2020-08-20 08:34:39 pgAdmin 4 commit: Added SQL Formatter support in Query Tool. Fixes #204