Обсуждение: Patch: add psql tab completion for event triggers
As it was kind of annoying not to have this when playing around with
event triggers.
This also tightens up the existing tab completion for ALTER TRIGGER,
which contained redundant code for table name completion, and which was
also causing a spurious "RENAME TO" to be inserted in this context:
CREATE EVENT TRIGGER foo ON {event} ^I
Regards
Ian Barwick
--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Вложения
On 08/04/14 18:22, Ian Barwick wrote:
> As it was kind of annoying not to have this when playing around with
> event triggers.
>
> This also tightens up the existing tab completion for ALTER TRIGGER,
> which contained redundant code for table name completion, and which was
> also causing a spurious "RENAME TO" to be inserted in this context:
>
> CREATE EVENT TRIGGER foo ON {event} ^I
Apologies, previous patch had some unrelated changes in it.
Correct patch attached.
--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Вложения
On Tue, Apr 8, 2014 at 5:27 AM, Ian Barwick <ian@2ndquadrant.com> wrote:
> On 08/04/14 18:22, Ian Barwick wrote:
>>
>> As it was kind of annoying not to have this when playing around with
>> event triggers.
>>
>> This also tightens up the existing tab completion for ALTER TRIGGER,
>> which contained redundant code for table name completion, and which was
>> also causing a spurious "RENAME TO" to be inserted in this context:
>>
>> CREATE EVENT TRIGGER foo ON {event} ^I
>
>
> Apologies, previous patch had some unrelated changes in it.
>
> Correct patch attached.
This *still* has some unrelated things in it, like s/Pgsql/Postgres/,
and numerous hunks consisting entirely of whitespace changes and/or
changes to unrelated comments.
Also, what's the point of this hunk:
*************** psql_completion(const char *text, int st
*** 1318,1340 **** pg_strcasecmp(prev2_wd, "TRIGGER") == 0)
COMPLETE_WITH_CONST("ON");
- else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
- pg_strcasecmp(prev3_wd, "TRIGGER") == 0)
- {
- completion_info_charp = prev2_wd;
- COMPLETE_WITH_QUERY(Query_for_list_of_tables_for_trigger);
- }
- /*
! * If we have ALTER TRIGGER <sth> ON, then add the correct tablename */ else if
(pg_strcasecmp(prev4_wd,"ALTER") == 0 && pg_strcasecmp(prev3_wd, "TRIGGER") == 0 &&
pg_strcasecmp(prev_wd, "ON") == 0)
! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
/* ALTER TRIGGER <name> ON <name> */
! else if (pg_strcasecmp(prev4_wd, "TRIGGER") == 0 && pg_strcasecmp(prev2_wd, "ON") == 0)
COMPLETE_WITH_CONST("RENAME TO");
--- 1355,1374 ---- pg_strcasecmp(prev2_wd, "TRIGGER") == 0)
COMPLETE_WITH_CONST("ON");
/*
! * If we have ALTER TRIGGER <name> ON, then add the correct tablename */ else if
(pg_strcasecmp(prev4_wd,"ALTER") == 0 && pg_strcasecmp(prev3_wd, "TRIGGER") == 0 &&
pg_strcasecmp(prev_wd, "ON") == 0)
! {
! completion_info_charp = prev2_wd;
! COMPLETE_WITH_QUERY(Query_for_list_of_tables_for_trigger);
! }
/* ALTER TRIGGER <name> ON <name> */
! else if (pg_strcasecmp(prev5_wd, "ALTER") == 0 &&
! pg_strcasecmp(prev4_wd, "TRIGGER") == 0 && pg_strcasecmp(prev2_wd,
"ON")== 0) COMPLETE_WITH_CONST("RENAME TO");
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 10/04/14 00:23, Robert Haas wrote:
> On Tue, Apr 8, 2014 at 5:27 AM, Ian Barwick <ian@2ndquadrant.com> wrote:
>> On 08/04/14 18:22, Ian Barwick wrote:
>>>
>>> As it was kind of annoying not to have this when playing around with
>>> event triggers.
>>>
>>> This also tightens up the existing tab completion for ALTER TRIGGER,
>>> which contained redundant code for table name completion, and which was
>>> also causing a spurious "RENAME TO" to be inserted in this context:
>>>
>>> CREATE EVENT TRIGGER foo ON {event} ^I
>>
>>
>> Apologies, previous patch had some unrelated changes in it.
>>
>> Correct patch attached.
>
> This *still* has some unrelated things in it, like s/Pgsql/Postgres/,
> and numerous hunks consisting entirely of whitespace changes and/or
> changes to unrelated comments.
Apologies again, that was ill-thought out. Revised patch attached with
only the additions related to event triggers, and the small fix for
ALTER TRIGGER mentioned above which ensures "RENAME TO" is applied only
when "ALTER TRIGGER <name> ON <sth>" was input; currently there is no
check for a preceding "ALTER", resulting in the spurious "RENAME TO"
when completing "CREATE EVENT TRIGGER".
> Also, what's the point of this hunk:
>
> *************** psql_completion(const char *text, int st
> *** 1318,1340 ****
> pg_strcasecmp(prev2_wd, "TRIGGER") == 0)
> COMPLETE_WITH_CONST("ON");
>
> - else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
> - pg_strcasecmp(prev3_wd, "TRIGGER") == 0)
> - {
> - completion_info_charp = prev2_wd;
> - COMPLETE_WITH_QUERY(Query_for_list_of_tables_for_trigger);
> - }
> -
> /*
> ! * If we have ALTER TRIGGER <sth> ON, then add the correct tablename
> */
> else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
> pg_strcasecmp(prev3_wd, "TRIGGER") == 0 &&
> pg_strcasecmp(prev_wd, "ON") == 0)
> ! COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
>
> /* ALTER TRIGGER <name> ON <name> */
> ! else if (pg_strcasecmp(prev4_wd, "TRIGGER") == 0 &&
> pg_strcasecmp(prev2_wd, "ON") == 0)
> COMPLETE_WITH_CONST("RENAME TO");
>
> --- 1355,1374 ----
> pg_strcasecmp(prev2_wd, "TRIGGER") == 0)
> COMPLETE_WITH_CONST("ON");
>
> /*
> ! * If we have ALTER TRIGGER <name> ON, then add the correct tablename
> */
> else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
> pg_strcasecmp(prev3_wd, "TRIGGER") == 0 &&
> pg_strcasecmp(prev_wd, "ON") == 0)
> ! {
> ! completion_info_charp = prev2_wd;
> ! COMPLETE_WITH_QUERY(Query_for_list_of_tables_for_trigger);
> ! }
>
> /* ALTER TRIGGER <name> ON <name> */
> ! else if (pg_strcasecmp(prev5_wd, "ALTER") == 0 &&
> ! pg_strcasecmp(prev4_wd, "TRIGGER") == 0 &&
> pg_strcasecmp(prev2_wd, "ON") == 0)
> COMPLETE_WITH_CONST("RENAME TO");
I'll submit that as a separate patch. This was intended to fix this:
else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
pg_strcasecmp(prev3_wd, "TRIGGER") == 0)
{
completion_info_charp = prev2_wd;
COMPLETE_WITH_QUERY(Query_for_list_of_tables_for_trigger);
}
/*
* If we have ALTER TRIGGER <sth> ON, then add the correct tablename
*/
else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
pg_strcasecmp(prev3_wd, "TRIGGER") == 0 &&
pg_strcasecmp(prev_wd, "ON") == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
as the second "else if" clause is redundant.
Regards
Ian Barwick
--
Ian Barwick http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Вложения
On Wed, Apr 9, 2014 at 8:58 PM, Ian Barwick <ian@2ndquadrant.com> wrote: > Apologies again, that was ill-thought out. Revised patch attached with only > the additions related to event triggers, and the small fix for ALTER TRIGGER > mentioned above which ensures "RENAME TO" is applied only when "ALTER > TRIGGER <name> ON <sth>" was input; currently there is no check for a > preceding "ALTER", resulting in the spurious "RENAME TO" when completing > "CREATE EVENT TRIGGER". OK, committed. (I know this was submitted rather late, but I think we've often allowed tab-completion fixups at similar times in past releases, since they are quite mechanical. If anyone feels that I shouldn't have committed this, please advise.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, Apr 14, 2014 at 9:46 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Wed, Apr 9, 2014 at 8:58 PM, Ian Barwick <ian@2ndquadrant.com> wrote: >> Apologies again, that was ill-thought out. Revised patch attached with only >> the additions related to event triggers, and the small fix for ALTER TRIGGER >> mentioned above which ensures "RENAME TO" is applied only when "ALTER >> TRIGGER <name> ON <sth>" was input; currently there is no check for a >> preceding "ALTER", resulting in the spurious "RENAME TO" when completing >> "CREATE EVENT TRIGGER". > > OK, committed. > > (I know this was submitted rather late, but I think we've often > allowed tab-completion fixups at similar times in past releases, since > they are quite mechanical. If anyone feels that I shouldn't have > committed this, please advise.) +1 for this change even if it came late in-game. Patch is straight-forward, and this is always useful to have. -- Michael