psql: tab completions for 'WITH'

Started by Josh Kupershmidtalmost 14 years ago4 messages
#1Josh Kupershmidt
schmiddy@gmail.com
2 attachment(s)

Hi all,

I noticed psql's tab-completion for 'WITH' is a bit overeager. If you
try to tab-complete commands like:
ALTER ROLE jsmith WITH [TAB]
COPY tbl FROM 'filename' WITH [TAB]

you'll get 'RECURSIVE' unhelpfully filled in. I think 'RECURSIVE'
should only be suggested if 'WITH' is the first and only word of the
line.

On a related note, I found it annoying that after fixing the above
problem, trying:
ALTER ROLE jsmith WITH [TAB]
CREATE ROLE jsmith WITH [TAB]

didn't suggest any tab-completions -- it only works if you leave off
the 'WITH' noise word, which I happen to use.

Attached are fixes for both of these gripes. I'll add to the next CF.

Josh

Attachments:

overeager_with.diffapplication/octet-stream; name=overeager_with.diffDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index 975d655..8ad5c4c
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 2826,2833 ****
  			  pg_strcasecmp(prev2_wd, "ANALYZE") == 0))
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
  
! /* WITH [RECURSIVE] */
! 	else if (pg_strcasecmp(prev_wd, "WITH") == 0)
  		COMPLETE_WITH_CONST("RECURSIVE");
  
  /* ANALYZE */
--- 2835,2845 ----
  			  pg_strcasecmp(prev2_wd, "ANALYZE") == 0))
  		COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
  
! /* WITH [RECURSIVE]. Only match when WITH is the first word, as WITH
!    may appear in various other contexts e.g. ALTER ROLE.
!  */
! 	else if (pg_strcasecmp(prev_wd, "WITH") == 0 &&
! 		     prev2_wd[0] == '\0')
  		COMPLETE_WITH_CONST("RECURSIVE");
  
  /* ANALYZE */
create_alter_role_tab_complete.diffapplication/octet-stream; name=create_alter_role_tab_complete.diffDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index 975d655..90bbfb7
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 999,1015 ****
  	}
  
  	/* ALTER USER,ROLE <name> */
! 	else if (pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
! 			 !(pg_strcasecmp(prev2_wd, "USER") == 0 && pg_strcasecmp(prev_wd, "MAPPING") == 0) &&
! 			 (pg_strcasecmp(prev2_wd, "USER") == 0 ||
! 			  pg_strcasecmp(prev2_wd, "ROLE") == 0))
  	{
  		static const char *const list_ALTERUSER[] =
  		{"CONNECTION LIMIT", "CREATEDB", "CREATEROLE", "CREATEUSER",
  			"ENCRYPTED", "INHERIT", "LOGIN", "NOCREATEDB", "NOCREATEROLE",
  			"NOCREATEUSER", "NOINHERIT", "NOLOGIN", "NOREPLICATION",
  			"NOSUPERUSER", "RENAME TO", "REPLICATION", "RESET", "SET",
! 		"SUPERUSER", "UNENCRYPTED", "VALID UNTIL", NULL};
  
  		COMPLETE_WITH_LIST(list_ALTERUSER);
  	}
--- 999,1019 ----
  	}
  
  	/* ALTER USER,ROLE <name> */
! 	else if ((pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
! 			  !(pg_strcasecmp(prev2_wd, "USER") == 0 && pg_strcasecmp(prev_wd, "MAPPING") == 0) &&
! 			  (pg_strcasecmp(prev2_wd, "USER") == 0 ||
! 			   pg_strcasecmp(prev2_wd, "ROLE") == 0)) ||
! 			 (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
! 			  (pg_strcasecmp(prev3_wd, "USER") == 0 ||
! 			   pg_strcasecmp(prev3_wd, "ROLE") == 0) &&
! 			  pg_strcasecmp(prev_wd, "WITH") == 0))
  	{
  		static const char *const list_ALTERUSER[] =
  		{"CONNECTION LIMIT", "CREATEDB", "CREATEROLE", "CREATEUSER",
  			"ENCRYPTED", "INHERIT", "LOGIN", "NOCREATEDB", "NOCREATEROLE",
  			"NOCREATEUSER", "NOINHERIT", "NOLOGIN", "NOREPLICATION",
  			"NOSUPERUSER", "RENAME TO", "REPLICATION", "RESET", "SET",
! 		 "SUPERUSER", "UNENCRYPTED", "VALID UNTIL", "WITH", NULL};
  
  		COMPLETE_WITH_LIST(list_ALTERUSER);
  	}
*************** psql_completion(char *text, int start, i
*** 1933,1949 ****
  		COMPLETE_WITH_CONST("PROCEDURE");
  
  /* CREATE ROLE,USER,GROUP */
! 	else if (pg_strcasecmp(prev3_wd, "CREATE") == 0 &&
  			 !(pg_strcasecmp(prev2_wd, "USER") == 0 && pg_strcasecmp(prev_wd, "MAPPING") == 0) &&
  			 (pg_strcasecmp(prev2_wd, "ROLE") == 0 ||
! 			  pg_strcasecmp(prev2_wd, "GROUP") == 0 || pg_strcasecmp(prev2_wd, "USER") == 0))
  	{
  		static const char *const list_CREATEROLE[] =
  		{"ADMIN", "CONNECTION LIMIT", "CREATEDB", "CREATEROLE", "CREATEUSER",
  			"ENCRYPTED", "IN", "INHERIT", "LOGIN", "NOCREATEDB",
  			"NOCREATEROLE", "NOCREATEUSER", "NOINHERIT", "NOLOGIN",
  			"NOREPLICATION", "NOSUPERUSER", "REPLICATION", "ROLE",
! 		"SUPERUSER", "SYSID", "UNENCRYPTED", "VALID UNTIL", NULL};
  
  		COMPLETE_WITH_LIST(list_CREATEROLE);
  	}
--- 1937,1958 ----
  		COMPLETE_WITH_CONST("PROCEDURE");
  
  /* CREATE ROLE,USER,GROUP */
! 	else if ((pg_strcasecmp(prev3_wd, "CREATE") == 0 &&
  			 !(pg_strcasecmp(prev2_wd, "USER") == 0 && pg_strcasecmp(prev_wd, "MAPPING") == 0) &&
  			 (pg_strcasecmp(prev2_wd, "ROLE") == 0 ||
! 			  pg_strcasecmp(prev2_wd, "GROUP") == 0 || pg_strcasecmp(prev2_wd, "USER") == 0)) ||
! 			 (pg_strcasecmp(prev4_wd, "CREATE") == 0 &&
! 			  (pg_strcasecmp(prev3_wd, "ROLE") == 0 ||
! 			   pg_strcasecmp(prev3_wd, "GROUP") == 0 ||
! 			   pg_strcasecmp(prev3_wd, "USER") == 0) &&
! 			   pg_strcasecmp(prev_wd, "WITH") == 0))
  	{
  		static const char *const list_CREATEROLE[] =
  		{"ADMIN", "CONNECTION LIMIT", "CREATEDB", "CREATEROLE", "CREATEUSER",
  			"ENCRYPTED", "IN", "INHERIT", "LOGIN", "NOCREATEDB",
  			"NOCREATEROLE", "NOCREATEUSER", "NOINHERIT", "NOLOGIN",
  			"NOREPLICATION", "NOSUPERUSER", "REPLICATION", "ROLE",
! 		 "SUPERUSER", "SYSID", "UNENCRYPTED", "VALID UNTIL", "WITH", NULL};
  
  		COMPLETE_WITH_LIST(list_CREATEROLE);
  	}
#2Peter Eisentraut
peter_e@gmx.net
In reply to: Josh Kupershmidt (#1)
Re: psql: tab completions for 'WITH'

On tis, 2012-04-03 at 22:34 -0700, Josh Kupershmidt wrote:

I noticed psql's tab-completion for 'WITH' is a bit overeager. If you
try to tab-complete commands like:
ALTER ROLE jsmith WITH [TAB]
COPY tbl FROM 'filename' WITH [TAB]

you'll get 'RECURSIVE' unhelpfully filled in. I think 'RECURSIVE'
should only be suggested if 'WITH' is the first and only word of the
line.

Committed that.

On a related note, I found it annoying that after fixing the above
problem, trying:
ALTER ROLE jsmith WITH [TAB]
CREATE ROLE jsmith WITH [TAB]

didn't suggest any tab-completions -- it only works if you leave off
the 'WITH' noise word, which I happen to use.

Hmm, but now you've set it up so that you can complete ALTER ROLE foo
WITH WITH. Were you aware of that?

#3Josh Kupershmidt
schmiddy@gmail.com
In reply to: Peter Eisentraut (#2)
1 attachment(s)
Re: psql: tab completions for 'WITH'

On Tue, Apr 10, 2012 at 10:38 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

On tis, 2012-04-03 at 22:34 -0700, Josh Kupershmidt wrote:

I noticed psql's tab-completion for 'WITH' is a bit overeager. If you
try to tab-complete commands like:
  ALTER ROLE jsmith WITH [TAB]
  COPY tbl FROM 'filename' WITH [TAB]

you'll get 'RECURSIVE' unhelpfully filled in. I think 'RECURSIVE'
should only be suggested if 'WITH' is the first and only word of the
line.

Committed that.

Thanks!

On a related note, I found it annoying that after fixing the above
problem, trying:
    ALTER ROLE jsmith WITH [TAB]
    CREATE ROLE jsmith WITH [TAB]

didn't suggest any tab-completions -- it only works if you leave off
the 'WITH' noise word, which I happen to use.

Hmm, but now you've set it up so that you can complete ALTER ROLE foo
WITH WITH.  Were you aware of that?

D'oh, I overlooked that. Attached is v2: the diff is a tad lengthier
now, but that should fix it.

Josh

Attachments:

create_alter_role_tab_complete.v2.diffapplication/octet-stream; name=create_alter_role_tab_complete.v2.diffDownload
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
new file mode 100644
index 00df2c6..5098a79
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** psql_completion(char *text, int start, i
*** 1024,1034 ****
  			"ENCRYPTED", "INHERIT", "LOGIN", "NOCREATEDB", "NOCREATEROLE",
  			"NOCREATEUSER", "NOINHERIT", "NOLOGIN", "NOREPLICATION",
  			"NOSUPERUSER", "RENAME TO", "REPLICATION", "RESET", "SET",
! 		"SUPERUSER", "UNENCRYPTED", "VALID UNTIL", NULL};
  
  		COMPLETE_WITH_LIST(list_ALTERUSER);
  	}
  
  	/* complete ALTER USER,ROLE <name> ENCRYPTED,UNENCRYPTED with PASSWORD */
  	else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
  			 (pg_strcasecmp(prev3_wd, "ROLE") == 0 || pg_strcasecmp(prev3_wd, "USER") == 0) &&
--- 1024,1051 ----
  			"ENCRYPTED", "INHERIT", "LOGIN", "NOCREATEDB", "NOCREATEROLE",
  			"NOCREATEUSER", "NOINHERIT", "NOLOGIN", "NOREPLICATION",
  			"NOSUPERUSER", "RENAME TO", "REPLICATION", "RESET", "SET",
! 		 "SUPERUSER", "UNENCRYPTED", "VALID UNTIL", "WITH", NULL};
  
  		COMPLETE_WITH_LIST(list_ALTERUSER);
  	}
  
+ 	/* ALTER USER,ROLE <name> WITH */
+ 	else if ((pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
+ 			  (pg_strcasecmp(prev3_wd, "USER") == 0 ||
+ 			   pg_strcasecmp(prev3_wd, "ROLE") == 0) &&
+ 			  pg_strcasecmp(prev_wd, "WITH") == 0))
+ 	{
+ 		/* Similar to the above, but don't offer "WITH" as a suggestion. */
+ 		static const char *const list_ALTERUSER_WITH[] =
+ 		{"CONNECTION LIMIT", "CREATEDB", "CREATEROLE", "CREATEUSER",
+ 			"ENCRYPTED", "INHERIT", "LOGIN", "NOCREATEDB", "NOCREATEROLE",
+ 			"NOCREATEUSER", "NOINHERIT", "NOLOGIN", "NOREPLICATION",
+ 			"NOSUPERUSER", "RENAME TO", "REPLICATION", "RESET", "SET",
+ 		 "SUPERUSER", "UNENCRYPTED", "VALID UNTIL", NULL};
+ 
+ 		COMPLETE_WITH_LIST(list_ALTERUSER_WITH);
+ 	}
+ 
  	/* complete ALTER USER,ROLE <name> ENCRYPTED,UNENCRYPTED with PASSWORD */
  	else if (pg_strcasecmp(prev4_wd, "ALTER") == 0 &&
  			 (pg_strcasecmp(prev3_wd, "ROLE") == 0 || pg_strcasecmp(prev3_wd, "USER") == 0) &&
*************** psql_completion(char *text, int start, i
*** 1947,1953 ****
  			 prev2_wd[0] != '\0')
  		COMPLETE_WITH_CONST("PROCEDURE");
  
! /* CREATE ROLE,USER,GROUP */
  	else if (pg_strcasecmp(prev3_wd, "CREATE") == 0 &&
  			 !(pg_strcasecmp(prev2_wd, "USER") == 0 && pg_strcasecmp(prev_wd, "MAPPING") == 0) &&
  			 (pg_strcasecmp(prev2_wd, "ROLE") == 0 ||
--- 1964,1970 ----
  			 prev2_wd[0] != '\0')
  		COMPLETE_WITH_CONST("PROCEDURE");
  
! /* CREATE ROLE,USER,GROUP <name> */
  	else if (pg_strcasecmp(prev3_wd, "CREATE") == 0 &&
  			 !(pg_strcasecmp(prev2_wd, "USER") == 0 && pg_strcasecmp(prev_wd, "MAPPING") == 0) &&
  			 (pg_strcasecmp(prev2_wd, "ROLE") == 0 ||
*************** psql_completion(char *text, int start, i
*** 1958,1968 ****
  			"ENCRYPTED", "IN", "INHERIT", "LOGIN", "NOCREATEDB",
  			"NOCREATEROLE", "NOCREATEUSER", "NOINHERIT", "NOLOGIN",
  			"NOREPLICATION", "NOSUPERUSER", "REPLICATION", "ROLE",
! 		"SUPERUSER", "SYSID", "UNENCRYPTED", "VALID UNTIL", NULL};
  
  		COMPLETE_WITH_LIST(list_CREATEROLE);
  	}
  
  	/*
  	 * complete CREATE ROLE,USER,GROUP <name> ENCRYPTED,UNENCRYPTED with
  	 * PASSWORD
--- 1975,2003 ----
  			"ENCRYPTED", "IN", "INHERIT", "LOGIN", "NOCREATEDB",
  			"NOCREATEROLE", "NOCREATEUSER", "NOINHERIT", "NOLOGIN",
  			"NOREPLICATION", "NOSUPERUSER", "REPLICATION", "ROLE",
! 		 "SUPERUSER", "SYSID", "UNENCRYPTED", "VALID UNTIL", "WITH", NULL};
  
  		COMPLETE_WITH_LIST(list_CREATEROLE);
  	}
  
+ /* CREATE ROLE,USER,GROUP <name> WITH */
+ 	else if ((pg_strcasecmp(prev4_wd, "CREATE") == 0 &&
+ 			  (pg_strcasecmp(prev3_wd, "ROLE") == 0 ||
+ 			   pg_strcasecmp(prev3_wd, "GROUP") == 0 ||
+ 			   pg_strcasecmp(prev3_wd, "USER") == 0) &&
+ 			  pg_strcasecmp(prev_wd, "WITH") == 0))
+ 	{
+ 		/* Similar to the above, but don't offer "WITH" as a suggestion. */
+ 		static const char *const list_CREATEROLE_WITH[] =
+ 		{"ADMIN", "CONNECTION LIMIT", "CREATEDB", "CREATEROLE", "CREATEUSER",
+ 			"ENCRYPTED", "IN", "INHERIT", "LOGIN", "NOCREATEDB",
+ 			"NOCREATEROLE", "NOCREATEUSER", "NOINHERIT", "NOLOGIN",
+ 			"NOREPLICATION", "NOSUPERUSER", "REPLICATION", "ROLE",
+ 		 "SUPERUSER", "SYSID", "UNENCRYPTED", "VALID UNTIL", NULL};
+ 
+ 		COMPLETE_WITH_LIST(list_CREATEROLE_WITH);
+ 	}
+ 
  	/*
  	 * complete CREATE ROLE,USER,GROUP <name> ENCRYPTED,UNENCRYPTED with
  	 * PASSWORD
#4Peter Eisentraut
peter_e@gmx.net
In reply to: Josh Kupershmidt (#3)
Re: psql: tab completions for 'WITH'

On tis, 2012-04-10 at 17:48 -0700, Josh Kupershmidt wrote:

Hmm, but now you've set it up so that you can complete ALTER ROLE

foo

WITH WITH. Were you aware of that?

D'oh, I overlooked that. Attached is v2: the diff is a tad lengthier
now, but that should fix it.

Committed.