Treat case of tab-completion keywords a bit more carefully.

When completing keywords that are offered alongside names obtained
from a query, preserve the user's choice of keyword case.  This
would have been messy to do before 02b8048ba, but now it's fairly
simple.  A complication is that we want keywords to be shown in
upper case in any tab-completion menus that include both keywords
and non-keywords, so we can't switch their case until enough has
been typed that only keyword(s) remain to be chosen.

Also, adjust some places where 02b8048ba thoughtlessly held over
a previous choice to display keywords in lower case.  (I think
I got confused as to whether those words were keywords or variable
names, but they're the former.)

Dagfinn Ilmari Mannsåker and Tom Lane

Discussion: https://postgr.es/m/8735l41ynm.fsf@wibble.ilmari.org
This commit is contained in:
Tom Lane 2022-02-01 17:05:09 -05:00
parent a5a9d77b8b
commit 020258fbd3
2 changed files with 61 additions and 28 deletions

View File

@ -40,7 +40,7 @@ $node->start;
# set up a few database objects # set up a few database objects
$node->safe_psql('postgres', $node->safe_psql('postgres',
"CREATE TABLE tab1 (f1 int primary key, f2 text);\n" "CREATE TABLE tab1 (c1 int primary key, c2 text);\n"
. "CREATE TABLE mytab123 (f1 int, f2 text);\n" . "CREATE TABLE mytab123 (f1 int, f2 text);\n"
. "CREATE TABLE mytab246 (f1 int, f2 text);\n" . "CREATE TABLE mytab246 (f1 int, f2 text);\n"
. "CREATE TABLE \"mixedName\" (f1 int, f2 text);\n" . "CREATE TABLE \"mixedName\" (f1 int, f2 text);\n"
@ -317,14 +317,30 @@ check_completion(
clear_line(); clear_line();
# check completion of a keyword offered in addition to object names # check completion of a keyword offered in addition to object names;
# (that code path currently doesn't preserve case of what's typed) # such a keyword should obey COMP_KEYWORD_CASE once only keyword
check_completion( # completions are possible
"comment on constraint foo on dom\t", foreach (
qr|DOMAIN|, [ 'lower', 'CO', 'column' ],
"offer keyword in addition to query result"); [ 'upper', 'co', 'COLUMN' ],
[ 'preserve-lower', 'co', 'column' ],
[ 'preserve-upper', 'CO', 'COLUMN' ],)
{
my ($case, $in, $out) = @$_;
clear_query(); check_completion(
"\\set COMP_KEYWORD_CASE $case\n",
qr/postgres=#/,
"set completion case to '$case'");
check_completion("alter table tab1 rename c\t\t",
qr|COLUMN|,
"offer keyword COLUMN for input c<TAB>, COMP_KEYWORD_CASE = $case");
clear_query();
check_completion("alter table tab1 rename $in\t\t\t",
qr|$out|,
"offer keyword $out for input $in<TAB>, COMP_KEYWORD_CASE = $case");
clear_query();
}
# send psql an explicit \q to shut it down, else pty won't close properly # send psql an explicit \q to shut it down, else pty won't close properly
$timer->start(5); $timer->start(5);

View File

@ -1320,9 +1320,11 @@ initialize_readline(void)
rl_basic_word_break_characters = WORD_BREAKS; rl_basic_word_break_characters = WORD_BREAKS;
/* /*
* We should include '"' in rl_completer_quote_characters too, but that * Ideally we'd include '"' in rl_completer_quote_characters too, which
* will require some upgrades to how we handle quoted identifiers, so * should allow us to complete quoted identifiers that include spaces.
* that's for another day. * However, the library support for rl_completer_quote_characters is
* presently too inconsistent to want to mess with that. (Note in
* particular that libedit has this variable but completely ignores it.)
*/ */
rl_completer_quote_characters = "'"; rl_completer_quote_characters = "'";
@ -2059,7 +2061,7 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH("SET", "RESET"); COMPLETE_WITH("SET", "RESET");
else if (Matches("ALTER", "SYSTEM", "SET|RESET")) else if (Matches("ALTER", "SYSTEM", "SET|RESET"))
COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_alter_system_set_vars, COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_alter_system_set_vars,
"all"); "ALL");
else if (Matches("ALTER", "SYSTEM", "SET", MatchAny)) else if (Matches("ALTER", "SYSTEM", "SET", MatchAny))
COMPLETE_WITH("TO"); COMPLETE_WITH("TO");
/* ALTER VIEW <name> */ /* ALTER VIEW <name> */
@ -4039,16 +4041,18 @@ psql_completion(const char *text, int start, int end)
/* Complete with a variable name */ /* Complete with a variable name */
else if (TailMatches("SET|RESET") && !TailMatches("UPDATE", MatchAny, "SET")) else if (TailMatches("SET|RESET") && !TailMatches("UPDATE", MatchAny, "SET"))
COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_set_vars, COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_set_vars,
"constraints", "CONSTRAINTS",
"transaction", "TRANSACTION",
"session", "SESSION",
"role", "ROLE",
"tablespace", "TABLESPACE",
"all"); "ALL");
else if (Matches("SHOW")) else if (Matches("SHOW"))
COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_show_vars, COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_show_vars,
"session authorization", "SESSION AUTHORIZATION",
"all"); "ALL");
else if (Matches("SHOW", "SESSION"))
COMPLETE_WITH("AUTHORIZATION");
/* Complete "SET TRANSACTION" */ /* Complete "SET TRANSACTION" */
else if (Matches("SET", "TRANSACTION")) else if (Matches("SET", "TRANSACTION"))
COMPLETE_WITH("SNAPSHOT", "ISOLATION LEVEL", "READ", "DEFERRABLE", "NOT DEFERRABLE"); COMPLETE_WITH("SNAPSHOT", "ISOLATION LEVEL", "READ", "DEFERRABLE", "NOT DEFERRABLE");
@ -4742,7 +4746,8 @@ _complete_from_query(const char *simple_query,
{ {
static int list_index, static int list_index,
num_schema_only, num_schema_only,
num_other; num_query_other,
num_keywords;
static PGresult *result = NULL; static PGresult *result = NULL;
static bool non_empty_object; static bool non_empty_object;
static bool schemaquoted; static bool schemaquoted;
@ -4765,7 +4770,8 @@ _complete_from_query(const char *simple_query,
/* Reset static state, ensuring no memory leaks */ /* Reset static state, ensuring no memory leaks */
list_index = 0; list_index = 0;
num_schema_only = 0; num_schema_only = 0;
num_other = 0; num_query_other = 0;
num_keywords = 0;
PQclear(result); PQclear(result);
result = NULL; result = NULL;
@ -4986,7 +4992,10 @@ _complete_from_query(const char *simple_query,
/* In verbatim mode, we return all the items as-is */ /* In verbatim mode, we return all the items as-is */
if (verbatim) if (verbatim)
{
num_query_other++;
return pg_strdup(item); return pg_strdup(item);
}
/* /*
* In normal mode, a name requiring quoting will be returned only * In normal mode, a name requiring quoting will be returned only
@ -5007,7 +5016,7 @@ _complete_from_query(const char *simple_query,
if (item == NULL && nsp != NULL) if (item == NULL && nsp != NULL)
num_schema_only++; num_schema_only++;
else else
num_other++; num_query_other++;
return requote_identifier(nsp, item, schemaquoted, objectquoted); return requote_identifier(nsp, item, schemaquoted, objectquoted);
} }
@ -5031,8 +5040,12 @@ _complete_from_query(const char *simple_query,
list_index++; list_index++;
if (pg_strncasecmp(text, item, strlen(text)) == 0) if (pg_strncasecmp(text, item, strlen(text)) == 0)
{ {
num_other++; num_keywords++;
return pg_strdup(item); /* Match keyword case if we are returning only keywords */
if (num_schema_only == 0 && num_query_other == 0)
return pg_strdup_keyword_case(item, text);
else
return pg_strdup(item);
} }
} }
} }
@ -5049,8 +5062,12 @@ _complete_from_query(const char *simple_query,
list_index++; list_index++;
if (pg_strncasecmp(text, item, strlen(text)) == 0) if (pg_strncasecmp(text, item, strlen(text)) == 0)
{ {
num_other++; num_keywords++;
return pg_strdup(item); /* Match keyword case if we are returning only keywords */
if (num_schema_only == 0 && num_query_other == 0)
return pg_strdup_keyword_case(item, text);
else
return pg_strdup(item);
} }
} }
} }
@ -5062,7 +5079,7 @@ _complete_from_query(const char *simple_query,
* completion subject text, which is not what we want. * completion subject text, which is not what we want.
*/ */
#ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER #ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER
if (num_schema_only > 0 && num_other == 0) if (num_schema_only > 0 && num_query_other == 0 && num_keywords == 0)
rl_completion_append_character = '\0'; rl_completion_append_character = '\0';
#endif #endif