Re: tab-completion debug print - Mailing list pgsql-hackers

Thank you for the comments.

At Sun, 25 Nov 2018 01:17:27 +0100, David Fetter <david@fetter.org> wrote in <20181125001727.GM958@fetter.org>
> On Fri, Nov 23, 2018 at 04:32:31PM -0500, Tom Lane wrote:
> > Hm.  I can see the value of instrumenting tab-complete when you're trying
> > to debug why it did something, but this output format seems pretty terse
> > and unreadable.  Can we get it to print the completion text as well?
> > I'm imagining something more like
> > 
> > 1414: "er "
> > 1435: ""
> > 1435: "ab"
> > 1435: ""
> > 1431: ""
> > 
> > Perhaps there's room as well to print the context that the match looked
> > at:
> > 
> > 1414: "alt" -> "er "
> > 1435: "alter " -> ""
> > 1435: "alter t" -> "ab"
> > 
> > etc.
> > 
> >             regards, tom lane
> 
> Is this something along the lines of what you had in mind?

The reason for the minimal output was it won't disturb console so
much when stderr is not redirected. It could be richer if we
premise redirection. Since every debug line is followed by a
line-feed, we don't need the brackets. Since the result is a set
so Devid's proposal to enclose the set by parentheses looks nice.
Also, result can contain matches containing word breaks (or even
an empty string) so quoting make every candidate string more
distinctive.

Finally, the attached shows the following debugging output.


INPUT: alter u<tab>(ser)<tab> <tab>
OUTPUT:
1445: "u" -> ("user", "user", "user mapping for")
1445: "user" -> ("user", "user", "user mapping for")
3630: "" -> ("", "horiguti", "MAPPING FOR", "pg_execute_server_program", "pg_monitor", "pg_read_all_settings",
"pg_read_all_stats","pg_read_server_files", "pg_signal_backend", "pg_stat_scan_tables", "pg_write_server_files",
"usr1","usr2")
 

INPUT: create function <tab>
OUTPUT:
3635: "" -> ("", "pg_ndistinct_out", "path_recv", "int4eq", "spg_quad_picksplit", "tsvectorsend", "float8_var_pop",
"opaque_in","interval_le", "macaddr_cmp", "range_gt..(17272 chars)..nge_in")
 

Wouldn't be a problem with the long line since it is for
debugging purpose. Of course we can limit the length easily (by
number of elements).

>+    for (int i = 0; list && list[i]; ++i)

# Are we using C99 for-loop-variable-declarations?

An arguable change in this version is enclosing empty result
lists with brackets, since it is just an intermediate state for
us.

[1431: "sael" -> ()]
3655: "sael" -> ("")

Showing completion context makes things a bit bothersome, since
complete_from_variables needs to take additional two parameters.
It is already shown in console conversasion so I'm not sure it's
worth the trouble.

1872: (...table t1 alter [COn]) -> ("CONSTRAINT")

To be friendly with the CI, the second patch is attached as the
difference from the first patch.


Note: This is not registered in this  CF.

- v3-0001-Psql-completion-debug-print-feature.patch
  W/O context version

- v3-0002-Add-context-information-in-tab-completion-debug-prin.patch
  W context version

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
From 573d8352331d0fd2ee0b5f3147cfcdcf7f1d4afc Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Mon, 26 Nov 2018 12:05:35 +0900
Subject: [PATCH 1/2] Psql completion debug print feature

Sometimes we find it bothersome to identify the code that actually
made a tab-completion. Psql will show the line number and completion
result to stderr by defining TABCOMPLETION_DEBUG at comiple time with
this patch.
---
 src/bin/psql/tab-complete.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 9dbd555166..00aa730050 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -60,6 +60,40 @@ extern char *filename_completion_function();
 #define completion_matches rl_completion_matches
 #endif
 
+/*
+ * By enabling the following definition every completion attempt emits to
+ * stderr the details of every completion prefixed by the source line number
+ * where it is made. You can isolate them from console interaction by
+ * redirecting stderr into a file.
+ */
+#ifdef TABCOMPLETION_DEBUG
+#ifdef HAVE_RL_COMPLETION_MATCHES
+#define org_completion_matches rl_completion_matches
+#else
+#define org_completion_matches completion_matches
+#endif
+
+#undef completion_matches
+#define completion_matches(t, f) completion_debug(__LINE__, (t), (f))
+
+static char **completion_debug(int line,
+                               const char *text, rl_compentry_func_t *func)
+{
+    char **list = org_completion_matches(text, func);
+
+    /*
+     * enclose empty list with brackets since it is an intermediate state
+     * which is immediately followed by a non-empty list.
+     */
+    fprintf(stderr, "%s%d: \"%s\" -> (", list ? "" : "[", line, text);
+    for (int i = 0; list && list[i]; ++i)
+        fprintf(stderr, "%s\"%s\"", i ? ", " : "", list[i]);            
+    fprintf(stderr, ")%s\n", list ? "": "]");
+
+    return list;
+}
+#endif
+
 /* word break characters */
 #define WORD_BREAKS        "\t\n@$><=;|&{() "
 
-- 
2.16.3

From 4bc47d44fa01417d4052634a003293dc4c6f15ce Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horiguchi.kyotaro@lab.ntt.co.jp>
Date: Mon, 26 Nov 2018 12:06:10 +0900
Subject: [PATCH 2/2] Add context information in tab-completion debug print

Psql shows completion context instead of just the word to complete.
---
 src/bin/psql/tab-complete.c | 48 ++++++++++++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 00aa730050..3823a181ec 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -74,18 +74,37 @@ extern char *filename_completion_function();
 #endif
 
 #undef completion_matches
-#define completion_matches(t, f) completion_debug(__LINE__, (t), (f))
+#define completion_matches(text, func) \
+    completion_debug(__LINE__, (text), (func), \
+                     previous_words, previous_words_count)
 
-static char **completion_debug(int line,
-                               const char *text, rl_compentry_func_t *func)
+#define DEBUG_NCONTEXT 3
+
+static char **
+completion_debug(int line, const char *text, rl_compentry_func_t *func,
+                 char **previous_words, int previous_words_count)
 {
     char **list = org_completion_matches(text, func);
+    int nwords = DEBUG_NCONTEXT;
 
     /*
      * enclose empty list with brackets since it is an intermediate state
      * which is immediately followed by a non-empty list.
      */
-    fprintf(stderr, "%s%d: \"%s\" -> (", list ? "" : "[", line, text);
+    fprintf(stderr, "%s%d: ", list ? "" : "[", line);
+
+    fputc('(', stderr);
+    if (previous_words_count > 0)
+    {
+        if (previous_words_count > nwords)
+            fprintf(stderr, "...");
+        else
+            nwords = previous_words_count;
+        for (int i = 0 ; i < nwords ; ++i)
+            fprintf(stderr, "%s ", previous_words[nwords - i - 1]);
+    }
+    fprintf(stderr, "[%s]) -> (", text);
+
     for (int i = 0; list && list[i]; ++i)
         fprintf(stderr, "%s\"%s\"", i ? ", " : "", list[i]);            
     fprintf(stderr, ")%s\n", list ? "": "]");
@@ -1046,7 +1065,8 @@ static void append_variable_names(char ***varnames, int *nvars,
                       int *maxvars, const char *varname,
                       const char *prefix, const char *suffix);
 static char **complete_from_variables(const char *text,
-                        const char *prefix, const char *suffix, bool need_value);
+                      const char *prefix, const char *suffix, bool need_value,
+                      char ** previous_words, int previous_words_count);
 static char *complete_from_files(const char *text, int state);
 
 static char *pg_strdup_keyword_case(const char *s, const char *ref);
@@ -1419,11 +1439,14 @@ psql_completion(const char *text, int start, int end)
     else if (text[0] == ':' && text[1] != ':')
     {
         if (text[1] == '\'')
-            matches = complete_from_variables(text, ":'", "'", true);
+            matches = complete_from_variables(text, ":'", "'", true,
+                         previous_words, previous_words_count);
         else if (text[1] == '"')
-            matches = complete_from_variables(text, ":\"", "\"", true);
+            matches = complete_from_variables(text, ":\"", "\"", true,
+                         previous_words, previous_words_count);
         else
-            matches = complete_from_variables(text, ":", "", true);
+            matches = complete_from_variables(text, ":", "", true,
+                         previous_words, previous_words_count);
     }
 
     /* If no previous word, suggest one of the basic sql commands */
@@ -3583,9 +3606,11 @@ psql_completion(const char *text, int start, int end)
             COMPLETE_WITH_CS("single", "double");
     }
     else if (TailMatchesCS("\\unset"))
-        matches = complete_from_variables(text, "", "", true);
+        matches = complete_from_variables(text, "", "", true,
+                         previous_words, previous_words_count);
     else if (TailMatchesCS("\\set"))
-        matches = complete_from_variables(text, "", "", false);
+        matches = complete_from_variables(text, "", "", false,
+                         previous_words, previous_words_count);
     else if (TailMatchesCS("\\set", MatchAny))
     {
         if (TailMatchesCS("AUTOCOMMIT|ON_ERROR_STOP|QUIET|"
@@ -4137,7 +4162,8 @@ append_variable_names(char ***varnames, int *nvars,
  */
 static char **
 complete_from_variables(const char *text, const char *prefix, const char *suffix,
-                        bool need_value)
+                        bool need_value,
+                        char ** previous_words, int previous_words_count)
 {
     char      **matches;
     char      **varnames;
-- 
2.16.3