From 020258fbd30d37ddd03d0ec68264d1544f8d2838 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 1 Feb 2022 17:05:09 -0500 Subject: [PATCH] Treat case of tab-completion keywords a bit more carefully. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/bin/psql/t/010_tab_completion.pl | 32 ++++++++++++---- src/bin/psql/tab-complete.c | 57 ++++++++++++++++++---------- 2 files changed, 61 insertions(+), 28 deletions(-) diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl index c4f6552ac9..7a265e0676 100644 --- a/src/bin/psql/t/010_tab_completion.pl +++ b/src/bin/psql/t/010_tab_completion.pl @@ -40,7 +40,7 @@ $node->start; # set up a few database objects $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 mytab246 (f1 int, f2 text);\n" . "CREATE TABLE \"mixedName\" (f1 int, f2 text);\n" @@ -317,14 +317,30 @@ check_completion( clear_line(); -# check completion of a keyword offered in addition to object names -# (that code path currently doesn't preserve case of what's typed) -check_completion( - "comment on constraint foo on dom\t", - qr|DOMAIN|, - "offer keyword in addition to query result"); +# check completion of a keyword offered in addition to object names; +# such a keyword should obey COMP_KEYWORD_CASE once only keyword +# completions are possible +foreach ( + [ 'lower', 'CO', 'column' ], + [ '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, COMP_KEYWORD_CASE = $case"); + clear_query(); + check_completion("alter table tab1 rename $in\t\t\t", + qr|$out|, + "offer keyword $out for input $in, COMP_KEYWORD_CASE = $case"); + clear_query(); +} # send psql an explicit \q to shut it down, else pty won't close properly $timer->start(5); diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index b2ec50b4f2..0ee94d7362 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1320,9 +1320,11 @@ initialize_readline(void) rl_basic_word_break_characters = WORD_BREAKS; /* - * We should include '"' in rl_completer_quote_characters too, but that - * will require some upgrades to how we handle quoted identifiers, so - * that's for another day. + * Ideally we'd include '"' in rl_completer_quote_characters too, which + * should allow us to complete quoted identifiers that include spaces. + * 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 = "'"; @@ -2059,7 +2061,7 @@ psql_completion(const char *text, int start, int end) COMPLETE_WITH("SET", "RESET"); else if (Matches("ALTER", "SYSTEM", "SET|RESET")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_alter_system_set_vars, - "all"); + "ALL"); else if (Matches("ALTER", "SYSTEM", "SET", MatchAny)) COMPLETE_WITH("TO"); /* ALTER VIEW */ @@ -4039,16 +4041,18 @@ psql_completion(const char *text, int start, int end) /* Complete with a variable name */ else if (TailMatches("SET|RESET") && !TailMatches("UPDATE", MatchAny, "SET")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_set_vars, - "constraints", - "transaction", - "session", - "role", - "tablespace", - "all"); + "CONSTRAINTS", + "TRANSACTION", + "SESSION", + "ROLE", + "TABLESPACE", + "ALL"); else if (Matches("SHOW")) COMPLETE_WITH_QUERY_PLUS(Query_for_list_of_show_vars, - "session authorization", - "all"); + "SESSION AUTHORIZATION", + "ALL"); + else if (Matches("SHOW", "SESSION")) + COMPLETE_WITH("AUTHORIZATION"); /* Complete "SET TRANSACTION" */ else if (Matches("SET", "TRANSACTION")) COMPLETE_WITH("SNAPSHOT", "ISOLATION LEVEL", "READ", "DEFERRABLE", "NOT DEFERRABLE"); @@ -4742,7 +4746,8 @@ _complete_from_query(const char *simple_query, { static int list_index, num_schema_only, - num_other; + num_query_other, + num_keywords; static PGresult *result = NULL; static bool non_empty_object; static bool schemaquoted; @@ -4765,7 +4770,8 @@ _complete_from_query(const char *simple_query, /* Reset static state, ensuring no memory leaks */ list_index = 0; num_schema_only = 0; - num_other = 0; + num_query_other = 0; + num_keywords = 0; PQclear(result); result = NULL; @@ -4986,7 +4992,10 @@ _complete_from_query(const char *simple_query, /* In verbatim mode, we return all the items as-is */ if (verbatim) + { + num_query_other++; return pg_strdup(item); + } /* * 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) num_schema_only++; else - num_other++; + num_query_other++; return requote_identifier(nsp, item, schemaquoted, objectquoted); } @@ -5031,8 +5040,12 @@ _complete_from_query(const char *simple_query, list_index++; if (pg_strncasecmp(text, item, strlen(text)) == 0) { - num_other++; - return pg_strdup(item); + num_keywords++; + /* 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++; if (pg_strncasecmp(text, item, strlen(text)) == 0) { - num_other++; - return pg_strdup(item); + num_keywords++; + /* 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. */ #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'; #endif