Re: Making tab-complete.c easier to maintain

I wrote:
> 3. The HeadMatches macros are pretty iffy because they can only look back
> nine words.  I'm tempted to redesign get_previous_words so it just
> tokenizes the whole line rather than having an arbitrary limitation.
> (For that matter, it's long overdue for it to be able to deal with
> multiline input...)

I poked into this and found that it's really not hard, if you don't mind
still another global variable associated with tab-completion.  See
attached patch.

The main objection to this change might be speed, but I experimented with
the longest query in information_schema.sql (CREATE VIEW usage_privileges,
162 lines) and found that pressing tab at the end of that seemed to take
well under a millisecond on my workstation.  So I think it's probably
insignificant, especially compared to any of the code paths that send a
query to the server for tab completion.

            regards, tom lane

diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c
index 2bc065a..ccd9a3e 100644
*** a/src/bin/psql/input.c
--- b/src/bin/psql/input.c
*************** static void finishInput(void);
*** 53,64 ****
   * gets_interactive()
   *
   * Gets a line of interactive input, using readline if desired.
   * The result is a malloc'd string.
   *
   * Caller *must* have set up sigint_interrupt_jmp before calling.
   */
  char *
! gets_interactive(const char *prompt)
  {
  #ifdef USE_READLINE
      if (useReadline)
--- 53,69 ----
   * gets_interactive()
   *
   * Gets a line of interactive input, using readline if desired.
+  *
+  * prompt: the prompt string to be used
+  * query_buf: buffer containing lines already read in the current command
+  * (query_buf is not modified here, but may be consulted for tab completion)
+  *
   * The result is a malloc'd string.
   *
   * Caller *must* have set up sigint_interrupt_jmp before calling.
   */
  char *
! gets_interactive(const char *prompt, PQExpBuffer query_buf)
  {
  #ifdef USE_READLINE
      if (useReadline)
*************** gets_interactive(const char *prompt)
*** 76,81 ****
--- 81,89 ----
          rl_reset_screen_size();
  #endif

+         /* Make current query_buf available to tab completion callback */
+         tab_completion_query_buf = query_buf;
+
          /* Enable SIGINT to longjmp to sigint_interrupt_jmp */
          sigint_interrupt_enabled = true;

*************** gets_interactive(const char *prompt)
*** 85,90 ****
--- 93,101 ----
          /* Disable SIGINT again */
          sigint_interrupt_enabled = false;

+         /* Pure neatnik-ism */
+         tab_completion_query_buf = NULL;
+
          return result;
      }
  #endif
diff --git a/src/bin/psql/input.h b/src/bin/psql/input.h
index abd7012..c9d30b9 100644
*** a/src/bin/psql/input.h
--- b/src/bin/psql/input.h
***************
*** 38,44 ****
  #include "pqexpbuffer.h"


! char       *gets_interactive(const char *prompt);
  char       *gets_fromFile(FILE *source);

  void        initializeInput(int flags);
--- 38,44 ----
  #include "pqexpbuffer.h"


! char       *gets_interactive(const char *prompt, PQExpBuffer query_buf);
  char       *gets_fromFile(FILE *source);

  void        initializeInput(int flags);
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index b6cef94..cb86457 100644
*** a/src/bin/psql/mainloop.c
--- b/src/bin/psql/mainloop.c
*************** MainLoop(FILE *source)
*** 133,139 ****
              /* May need to reset prompt, eg after \r command */
              if (query_buf->len == 0)
                  prompt_status = PROMPT_READY;
!             line = gets_interactive(get_prompt(prompt_status));
          }
          else
          {
--- 133,139 ----
              /* May need to reset prompt, eg after \r command */
              if (query_buf->len == 0)
                  prompt_status = PROMPT_READY;
!             line = gets_interactive(get_prompt(prompt_status), query_buf);
          }
          else
          {
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 731df2a..fec7c38 100644
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
*************** extern char *filename_completion_functio
*** 70,75 ****
--- 70,82 ----
  #define WORD_BREAKS        "\t\n@$><=;|&{() "

  /*
+  * Since readline doesn't let us pass any state through to the tab completion
+  * callback, we have to use this global variable to let get_previous_words()
+  * get at the previous lines of the current command.  Ick.
+  */
+ PQExpBuffer tab_completion_query_buf = NULL;
+
+ /*
   * This struct is used to define "schema queries", which are custom-built
   * to obtain possibly-schema-qualified names of database objects.  There is
   * enough similarity in the structure that we don't want to repeat it each
*************** static char *pg_strdup_keyword_case(cons
*** 923,929 ****
  static char *escape_string(const char *text);
  static PGresult *exec_query(const char *query);

! static int    get_previous_words(int point, char **previous_words, int nwords);

  static char *get_guctype(const char *varname);

--- 930,936 ----
  static char *escape_string(const char *text);
  static PGresult *exec_query(const char *query);

! static char **get_previous_words(int point, char **buffer, int *nwords);

  static char *get_guctype(const char *varname);

*************** psql_completion(const char *text, int st
*** 1027,1039 ****
      /* This is the variable we'll return. */
      char      **matches = NULL;

!     /* This array will contain some scannage of the input line. */
!     char       *previous_words[9];

      /* The number of words found on the input line. */
      int            previous_words_count;

!     /* For compactness, we use these macros to reference previous_words[]. */
  #define prev_wd   (previous_words[0])
  #define prev2_wd  (previous_words[1])
  #define prev3_wd  (previous_words[2])
--- 1034,1055 ----
      /* This is the variable we'll return. */
      char      **matches = NULL;

!     /* Workspace for parsed words. */
!     char       *words_buffer;
!
!     /* This array will contain pointers to parsed words. */
!     char      **previous_words;

      /* The number of words found on the input line. */
      int            previous_words_count;

!     /*
!      * For compactness, we use these macros to reference previous_words[].
!      * Caution: do not access a previous_words[] entry without having checked
!      * previous_words_count to be sure it's valid.  In most cases below, that
!      * check is implicit in a TailMatches() or similar macro, but in some
!      * places we have to check it explicitly.
!      */
  #define prev_wd   (previous_words[0])
  #define prev2_wd  (previous_words[1])
  #define prev3_wd  (previous_words[2])
*************** psql_completion(const char *text, int st
*** 1133,1141 ****

      /*
       * Macros for matching N words at the start of the line, regardless of
!      * what is after them.  These are pretty broken because they can only look
!      * back lengthof(previous_words) words, but we'll worry about improving
!      * their implementation some other day.
       */
  #define HeadMatches1(p1) \
      (previous_words_count >= 1 && \
--- 1149,1155 ----

      /*
       * Macros for matching N words at the start of the line, regardless of
!      * what is after them.
       */
  #define HeadMatches1(p1) \
      (previous_words_count >= 1 && \
*************** psql_completion(const char *text, int st
*** 1194,1206 ****
      completion_info_charp2 = NULL;

      /*
!      * Scan the input line before our current position for the last few words.
       * According to those we'll make some smart decisions on what the user is
       * probably intending to type.
       */
!     previous_words_count = get_previous_words(start,
!                                               previous_words,
!                                               lengthof(previous_words));

      /* If current word is a backslash command, offer completions for that */
      if (text[0] == '\\')
--- 1208,1220 ----
      completion_info_charp2 = NULL;

      /*
!      * Scan the input line to extract the words before our current position.
       * According to those we'll make some smart decisions on what the user is
       * probably intending to type.
       */
!     previous_words = get_previous_words(start,
!                                         &words_buffer,
!                                         &previous_words_count);

      /* If current word is a backslash command, offer completions for that */
      if (text[0] == '\\')
*************** psql_completion(const char *text, int st
*** 1692,1698 ****

          COMPLETE_WITH_LIST(list_TABLEOPTIONS);
      }
!     else if (TailMatches4("REPLICA", "IDENTITY", "USING", "INDEX"))
      {
          completion_info_charp = prev5_wd;
          COMPLETE_WITH_QUERY(Query_for_index_of_table);
--- 1706,1712 ----

          COMPLETE_WITH_LIST(list_TABLEOPTIONS);
      }
!     else if (TailMatches5(MatchAny, "REPLICA", "IDENTITY", "USING", "INDEX"))
      {
          completion_info_charp = prev5_wd;
          COMPLETE_WITH_QUERY(Query_for_index_of_table);
*************** psql_completion(const char *text, int st
*** 2806,2812 ****
          if (!recognized_connection_string(text))
              COMPLETE_WITH_QUERY(Query_for_list_of_databases);
      }
!     else if (strcmp(prev2_wd, "\\connect") == 0 || strcmp(prev2_wd, "\\c") == 0)
      {
          if (!recognized_connection_string(prev_wd))
              COMPLETE_WITH_QUERY(Query_for_list_of_roles);
--- 2820,2828 ----
          if (!recognized_connection_string(text))
              COMPLETE_WITH_QUERY(Query_for_list_of_databases);
      }
!     else if (previous_words_count >= 2 &&
!              (strcmp(prev2_wd, "\\connect") == 0 ||
!               strcmp(prev2_wd, "\\c") == 0))
      {
          if (!recognized_connection_string(prev_wd))
              COMPLETE_WITH_QUERY(Query_for_list_of_roles);
*************** psql_completion(const char *text, int st
*** 2891,2897 ****

          COMPLETE_WITH_LIST_CS(my_list);
      }
!     else if (strcmp(prev2_wd, "\\pset") == 0)
      {
          if (strcmp(prev_wd, "format") == 0)
          {
--- 2907,2914 ----

          COMPLETE_WITH_LIST_CS(my_list);
      }
!     else if (previous_words_count >= 2 &&
!              strcmp(prev2_wd, "\\pset") == 0)
      {
          if (strcmp(prev_wd, "format") == 0)
          {
*************** psql_completion(const char *text, int st
*** 2927,2933 ****
      {
          matches = complete_from_variables(text, "", "", false);
      }
!     else if (strcmp(prev2_wd, "\\set") == 0)
      {
          static const char *const boolean_value_list[] =
          {"on", "off", NULL};
--- 2944,2951 ----
      {
          matches = complete_from_variables(text, "", "", false);
      }
!     else if (previous_words_count >= 2 &&
!              strcmp(prev2_wd, "\\set") == 0)
      {
          static const char *const boolean_value_list[] =
          {"on", "off", NULL};
*************** psql_completion(const char *text, int st
*** 3048,3059 ****
      }

      /* free storage */
!     {
!         int            i;
!
!         for (i = 0; i < lengthof(previous_words); i++)
!             free(previous_words[i]);
!     }

      /* Return our Grand List O' Matches */
      return matches;
--- 3066,3073 ----
      }

      /* free storage */
!     free(previous_words);
!     free(words_buffer);

      /* Return our Grand List O' Matches */
      return matches;
*************** exec_query(const char *query)
*** 3632,3661 ****


  /*
!  * Return the nwords word(s) before point.  Words are returned right to left,
!  * that is, previous_words[0] gets the last word before point.
!  * If we run out of words, remaining array elements are set to empty strings.
!  * Each array element is filled with a malloc'd string.
!  * Returns the number of words that were actually found (up to nwords).
   */
! static int
! get_previous_words(int point, char **previous_words, int nwords)
  {
!     const char *buf = rl_line_buffer;    /* alias */
      int            words_found = 0;
      int            i;

!     /* first we look for a non-word char before the current point */
      for (i = point - 1; i >= 0; i--)
          if (strchr(WORD_BREAKS, buf[i]))
              break;
      point = i;

!     while (nwords-- > 0)
      {
          int            start,
                      end;
!         char       *s;

          /* now find the first non-space which then constitutes the end */
          end = -1;
--- 3646,3725 ----


  /*
!  * Parse all the word(s) before point.
!  *
!  * Returns a malloc'd array of character pointers that point into the malloc'd
!  * data array returned to *buffer; caller must free() both of these when done.
!  * *nwords receives the number of words found, ie, the valid length of the
!  * return array.
!  *
!  * Words are returned right to left, that is, previous_words[0] gets the last
!  * word before point, previous_words[1] the next-to-last, etc.
   */
! static char **
! get_previous_words(int point, char **buffer, int *nwords)
  {
!     char      **previous_words;
!     char       *buf;
!     int            buflen;
      int            words_found = 0;
      int            i;

!     /*
!      * Construct a writable buffer including both preceding and current lines
!      * of the query, up to "point" which is where the currently completable
!      * word begins.  Because our definition of "word" is such that a non-word
!      * character must end each word, we can use this buffer to return the word
!      * data as-is, by placing a '\0' after each word.
!      */
!     buflen = point + 1;
!     if (tab_completion_query_buf)
!         buflen += tab_completion_query_buf->len + 1;
!     *buffer = buf = pg_malloc(buflen);
!     i = 0;
!     if (tab_completion_query_buf)
!     {
!         memcpy(buf, tab_completion_query_buf->data,
!                tab_completion_query_buf->len);
!         i += tab_completion_query_buf->len;
!         buf[i++] = '\n';
!     }
!     memcpy(buf + i, rl_line_buffer, point);
!     i += point;
!     buf[i] = '\0';
!
!     /* Readjust point to reference appropriate offset in buf */
!     point = i;
!
!     /*
!      * Allocate array of word start points.  There can be at most length/2 + 1
!      * words in the buffer.
!      */
!     previous_words = (char **) pg_malloc((point / 2 + 1) * sizeof(char *));
!
!     /*
!      * First we look for a non-word char before the current point.  (This is
!      * probably useless, if readline is on the same page as we are about what
!      * is a word, but if so it's cheap.)
!      */
      for (i = point - 1; i >= 0; i--)
+     {
          if (strchr(WORD_BREAKS, buf[i]))
              break;
+     }
      point = i;

!     /*
!      * Now parse words, working backwards, until we hit start of line.  The
!      * backwards scan has some interesting but intentional properties
!      * concerning parenthesis handling.
!      */
!     while (point >= 0)
      {
          int            start,
                      end;
!         bool        inquotes = false;
!         int            parentheses = 0;

          /* now find the first non-space which then constitutes the end */
          end = -1;
*************** get_previous_words(int point, char **pre
*** 3667,3725 ****
                  break;
              }
          }

          /*
!          * If no end found we return an empty string, because there is no word
!          * before the point
           */
!         if (end < 0)
!         {
!             point = end;
!             s = pg_strdup("");
!         }
!         else
          {
!             /*
!              * Otherwise we now look for the start. The start is either the
!              * last character before any word-break character going backwards
!              * from the end, or it's simply character 0. We also handle open
!              * quotes and parentheses.
!              */
!             bool        inquotes = false;
!             int            parentheses = 0;
!
!             for (start = end; start > 0; start--)
              {
!                 if (buf[start] == '"')
!                     inquotes = !inquotes;
!                 if (!inquotes)
                  {
!                     if (buf[start] == ')')
!                         parentheses++;
!                     else if (buf[start] == '(')
!                     {
!                         if (--parentheses <= 0)
!                             break;
!                     }
!                     else if (parentheses == 0 &&
!                              strchr(WORD_BREAKS, buf[start - 1]))
                          break;
                  }
              }
-
-             point = start - 1;
-
-             /* make a copy of chars from start to end inclusive */
-             s = pg_malloc(end - start + 2);
-             strlcpy(s, &buf[start], end - start + 2);
-
-             words_found++;
          }

!         *previous_words++ = s;
      }

!     return words_found;
  }

  /*
--- 3731,3776 ----
                  break;
              }
          }
+         /* if no end found, we're done */
+         if (end < 0)
+             break;

          /*
!          * Otherwise we now look for the start.  The start is either the last
!          * character before any word-break character going backwards from the
!          * end, or it's simply character 0.  We also handle open quotes and
!          * parentheses.
           */
!         for (start = end; start > 0; start--)
          {
!             if (buf[start] == '"')
!                 inquotes = !inquotes;
!             if (!inquotes)
              {
!                 if (buf[start] == ')')
!                     parentheses++;
!                 else if (buf[start] == '(')
                  {
!                     if (--parentheses <= 0)
                          break;
                  }
+                 else if (parentheses == 0 &&
+                          strchr(WORD_BREAKS, buf[start - 1]))
+                     break;
              }
          }

!         /* Return the word located at start to end inclusive */
!         previous_words[words_found] = &buf[start];
!         buf[end + 1] = '\0';
!         words_found++;
!
!         /* Continue searching */
!         point = start - 1;
      }

!     *nwords = words_found;
!     return previous_words;
  }

  /*
diff --git a/src/bin/psql/tab-complete.h b/src/bin/psql/tab-complete.h
index 9dcd7e7..0e9a430 100644
*** a/src/bin/psql/tab-complete.h
--- b/src/bin/psql/tab-complete.h
***************
*** 8,15 ****
  #ifndef TAB_COMPLETE_H
  #define TAB_COMPLETE_H

! #include "postgres_fe.h"

! void        initialize_readline(void);

! #endif
--- 8,17 ----
  #ifndef TAB_COMPLETE_H
  #define TAB_COMPLETE_H

! #include "pqexpbuffer.h"

! extern PQExpBuffer tab_completion_query_buf;

! extern void initialize_readline(void);
!
! #endif   /* TAB_COMPLETE_H */