Re: Tab complete for CREATE OR REPLACE TRIGGER statement
Hello Tom, On 2020-11-18 16:49, Tom Lane wrote: Tels writes: On 2020-11-18 06:06, Michael Paquier wrote: On Mon, Nov 16, 2020 at 10:14:10PM -0500, Tom Lane wrote: Agreed, I'm not trying to block this patch. Just wishing there were a better way. To me the code looks like a prime candidate for "data-driven" refactoring. It should be redone as generic code that reads a table of rules with params and then checks and applies each. Thus the repetitive code would be replaced by a bit more generic code and a lot of code-free data. In the past I've looked into whether the rules could be autogenerated from the backend's grammar. It did not look very promising though. The grammar isn't really factorized appropriately -- for instance, tab-complete has a lot of knowledge about which kinds of objects can be named in particular places, while the Bison productions only know that's a "name". Still, the precedent of ECPG suggests it might be possible to process the grammar rules somehow to get to a useful result. Hm, that would be even better, for now I was just thinking that code like this: IF RULE_A_MATCHES THEN DO STUFF A ELSE IF RULE_B_MATCHES THEN DO_STUFF_B ELSE IF RULE_C_MATCHES THEN DO_STUFF_C ... should be replaced by RULE_A MATCH STUFF RULE_B MATCH STUFF RULE_C MATCH STUFF ... FOREACH RULE DO IF RULE.MATCH THEN DO RULE.STUFF END FOREACH Even if the rules would be manually created (converted from the current code), that would be more compact and probably less error-prone. Creating the rule automatically turns this into a completely different story. -- Best regards, Tels
Re: Tab complete for CREATE OR REPLACE TRIGGER statement
Tels writes: > On 2020-11-18 06:06, Michael Paquier wrote: >> On Mon, Nov 16, 2020 at 10:14:10PM -0500, Tom Lane wrote: >>> Agreed, I'm not trying to block this patch. Just wishing >>> there were a better way. > To me the code looks like a prime candidate for "data-driven" > refactoring. > It should be redone as generic code that reads a table of rules with > params and then checks and applies each. Thus the repetitive code would > be replaced by a bit more generic code and a lot of code-free data. In the past I've looked into whether the rules could be autogenerated from the backend's grammar. It did not look very promising though. The grammar isn't really factorized appropriately -- for instance, tab-complete has a lot of knowledge about which kinds of objects can be named in particular places, while the Bison productions only know that's a "name". Still, the precedent of ECPG suggests it might be possible to process the grammar rules somehow to get to a useful result. regards, tom lane
Re: Tab complete for CREATE OR REPLACE TRIGGER statement
On 2020-11-18 06:06, Michael Paquier wrote: On Mon, Nov 16, 2020 at 10:14:10PM -0500, Tom Lane wrote: Michael Paquier writes: I don't think that this is a requirement for this thread, though. Agreed, I'm not trying to block this patch. Just wishing there were a better way. Okay. I have tweaked a couple of things in the patch and applied it. I am wondering if libreadline gives the possibility to implement an optional grouping of words to complete, but diving into its documentation I have not found something that we could use. To me the code looks like a prime candidate for "data-driven" refactoring. It should be redone as generic code that reads a table of rules with params and then checks and applies each. Thus the repetitive code would be replaced by a bit more generic code and a lot of code-free data. -- Best regards, Tels
RE: Tab complete for CREATE OR REPLACE TRIGGER statement
> Okay. I have tweaked a couple of things in the patch and applied it. Thank you so much. The next time I post a patch, be careful with git --diff check and indentation. Regards, Noriyoshi Shinoda -Original Message- From: Michael Paquier [mailto:mich...@paquier.xyz] Sent: Wednesday, November 18, 2020 2:07 PM To: Tom Lane Cc: Shinoda, Noriyoshi (PN Japan FSI) ; pgsql-hackers@lists.postgresql.org Subject: Re: Tab complete for CREATE OR REPLACE TRIGGER statement On Mon, Nov 16, 2020 at 10:14:10PM -0500, Tom Lane wrote: > Michael Paquier writes: >> I don't think that this is a requirement for this thread, though. > > Agreed, I'm not trying to block this patch. Just wishing there were a > better way. Okay. I have tweaked a couple of things in the patch and applied it. I am wondering if libreadline gives the possibility to implement an optional grouping of words to complete, but diving into its documentation I have not found something that we could use. -- Michael
Re: Tab complete for CREATE OR REPLACE TRIGGER statement
On Mon, Nov 16, 2020 at 10:14:10PM -0500, Tom Lane wrote: > Michael Paquier writes: >> I don't think that this is a requirement for this thread, though. > > Agreed, I'm not trying to block this patch. Just wishing > there were a better way. Okay. I have tweaked a couple of things in the patch and applied it. I am wondering if libreadline gives the possibility to implement an optional grouping of words to complete, but diving into its documentation I have not found something that we could use. -- Michael signature.asc Description: PGP signature
Re: Tab complete for CREATE OR REPLACE TRIGGER statement
Michael Paquier writes: > I don't think that this is a requirement for this thread, though. Agreed, I'm not trying to block this patch. Just wishing there were a better way. regards, tom lane
Re: Tab complete for CREATE OR REPLACE TRIGGER statement
On Mon, Nov 16, 2020 at 09:14:51PM -0500, Tom Lane wrote: > It's kind of depressing how repetitive the patch is. There's > probably not much to be done about that in the short run, but > it seems to point up the need to start thinking about how to > refactor tab-complete.c more thoroughly. Agreed. One thing that I'd think could help is a new wild card to make some of the words conditional in the list of items. But that may be tricky once you consider the case of a group of words. I don't think that this is a requirement for this thread, though. -- Michael signature.asc Description: PGP signature
Re: Tab complete for CREATE OR REPLACE TRIGGER statement
Michael Paquier writes: > On Mon, Nov 16, 2020 at 08:12:05AM +, Shinoda, Noriyoshi (PN Japan FSI) > wrote: >> Yesterday, OR REPLACE clause was provided to the CREATE TRIGGER >> statement, so I wrote a patch for tab completion for the psql >> command. > Thanks, the logic looks fine. I'll apply if there are no objections. > Please note that git diff --check and that the indentation does not > seem quite right, but that's easy enough to fix. It's kind of depressing how repetitive the patch is. There's probably not much to be done about that in the short run, but it seems to point up the need to start thinking about how to refactor tab-complete.c more thoroughly. regards, tom lane
Re: Tab complete for CREATE OR REPLACE TRIGGER statement
On Mon, Nov 16, 2020 at 08:12:05AM +, Shinoda, Noriyoshi (PN Japan FSI) wrote: > Yesterday, OR REPLACE clause was provided to the CREATE TRIGGER > statement, so I wrote a patch for tab completion for the psql > command. Thanks, the logic looks fine. I'll apply if there are no objections. Please note that git diff --check and that the indentation does not seem quite right, but that's easy enough to fix. -- Michael signature.asc Description: PGP signature
Tab complete for CREATE OR REPLACE TRIGGER statement
Hi, Hackers. Yesterday, OR REPLACE clause was provided to the CREATE TRIGGER statement, so I wrote a patch for tab completion for the psql command. TRIGGER adds tab completion to the CREATE OR REPLACE statement, and the CREATE TRIGGER and CREATE OR REPLACE TRIGGER statements are completed in the same way. I referred to the tab completion code of the CREATE RULE statement. Regards, Noriyoshi Shinoda psql_create_or_replace_trigger_tab.diff Description: psql_create_or_replace_trigger_tab.diff
