From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Jeff Janes <jeff.janes@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Subject: Re: Making tab-complete.c easier to maintain
Date: Sat, 19 Dec 2015 18:36:10 -0500
Message-ID: <14995.1450568170@sss.pgh.pa.us> (raw)
In-Reply-To: <21460.1450560248@sss.pgh.pa.us>
References: <CAB7nPqQ-=c0P7A1cWAo7ONNEPfq4+aNpX86a+oHKS4bcGs2C6Q@mail.gmail.com>
<CAEepm=1x-inE_3Hsa07Wp5zDg2puGR1TdJsstgm0=ATXpxur=Q@mail.gmail.com>
<CAB7nPqSggjPA8U1WV7ivW44xzboC8pci_Etmffr+ZEzxSX_ahA@mail.gmail.com>
<20151217.150633.216859242.horiguchi.kyotaro@lab.ntt.co.jp>
<CAB7nPqS=UHm2QyJtx+kyyHb4WH09vK3NVUqJVFBotQAQx5hD4g@mail.gmail.com>
<CAEepm=2vMAESiiind-jbQK-sAibLr1440E8pj_KmWYO8XBtiRA@mail.gmail.com>
<1851.1450471370@sss.pgh.pa.us>
<CAB7nPqS0-TjcwUWM4R+-77aCrAz9B6a+S_d973CeJvfLm7r-Tg@mail.gmail.com>
<21460.1450560248@sss.pgh.pa.us>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersAttachments:multiline-tab-completion.patch (text/x-diff, 15.7K) downloaddiff --git a/src/bin/psql/input.c b/src/bin/psql/input.cindex 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.hindex 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.cindex 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.cindex 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.hindex 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 */
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, michael.paquier@gmail.com, thomas.munro@enterprisedb.com, horiguchi.kyotaro@lab.ntt.co.jp, alvherre@2ndquadrant.com, jeff.janes@gmail.com
Subject: Re: Making tab-complete.c easier to maintain
In-Reply-To: <14995.1450568170@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox