8000 Improve psql's tab completion for filenames. · postgrespro/postgres_cluster@cd69ec6 · GitHub
[go: up one dir, main page]
Skip to content

Commit cd69ec6

Browse files
tglsfdctglsfdc
committed
Improve psql's tab completion for filenames.
The Readline library contains a fair amount of knowledge about how to tab-complete filenames, but it turns out that that doesn't work too well unless we follow its expectation that we use its filename quoting hooks to quote and de-quote filenames. We were trying to do such quote handling within complete_from_files(), and that's still what we have to do if we're using libedit, which lacks those hooks. But for Readline, it works a lot better if we tell Readline that single-quote is a quoting character and then provide hooks that know the details of the quoting rules for SQL and psql meta-commands. Hence, resurrect the quoting hook functions that existed in the original version of tab-complete.c (and were disabled by commit f6689a3 because they "didn't work so well yet"), and whack on them until they do seem to work well. Notably, this fixes bug #16059 from Steven Winfield, who pointed out that the previous coding would strip quote marks from filenames in SQL COPY commands, even though they're syntactically necessary there. Now, we not only don't do that, but we'll add a quote mark when you tab-complete, even if you didn't type one. Getting this to work across a range of libedit versions (and, to a lesser extent, libreadline versions) was depressingly difficult. It will be interesting to see whether the new regression test cases pass everywhere in the buildfarm. Some future patch might try to handle quoted SQL identifiers with similar explicit quoting/dequoting logic, but that's for another day. Patch by me, reviewed by Peter Eisentraut. Discussion: https://postgr.es/m/16059-8836946734c02b84@postgresql.org
1 parent 5ba40b6 commit cd69ec6

File tree

9 files changed

+438
-55
lines changed

9 files changed

+438
-55
lines changed

‎config/programs.m4‎

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -209,17 +209,20 @@ fi
209209

210210

211211

212-
# PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
213-
# ---------------------------------------
212+
# PGAC_READLINE_VARIABLES
213+
# -----------------------
214214
# Readline versions < 2.1 don't have rl_completion_append_character
215+
# Libedit lacks rl_filename_quote_characters and rl_filename_quoting_function
215216

216-
AC_DEFUN([PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER],
217+
AC_DEFUN([PGAC_READLINE_VARIABLES],
217218
[AC_CACHE_CHECK([for rl_completion_append_character], pgac_cv_var_rl_completion_append_character,
218219
[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <stdio.h>
219-
#ifdef HAVE_READLINE_READLINE_H
220-
# include <readline/readline.h>
220+
#if defined(HAVE_READLINE_READLINE_H)
221+
#include <readline/readline.h>
222+
#elif defined(HAVE_EDITLINE_READLINE_H)
223+
#include <editline/readline.h>
221224
#elif defined(HAVE_READLINE_H)
222-
# include <readline.h>
225+
#include <readline.h>
223226
#endif
224227
],
225228
[rl_completion_append_character = 'x';])],
@@ -228,7 +231,42 @@ AC_DEFUN([PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER],
228231
if test x"$pgac_cv_var_rl_completion_append_character" = x"yes"; then
229232
AC_DEFINE(HAVE_RL_COMPLETION_APPEND_CHARACTER, 1,
230233
[Define to 1 if you have the global variable 'rl_completion_append_character'.])
231-
fi])# PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
234+
fi
235+
AC_CACHE_CHECK([for rl_filename_quote_characters], pgac_cv_var_rl_filename_quote_characters,
236+
[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <stdio.h>
237+
#if defined(HAVE_READLINE_READLINE_H)
238+
#include <readline/readline.h>
239+
#elif defined(HAVE_EDITLINE_READLINE_H)
240+
#include <editline/readline.h>
241+
#elif defined(HAVE_READLINE_H)
242+
#include <readline.h>
243+
#endif
244+
],
245+
[rl_filename_quote_characters = "x";])],
246+
[pgac_cv_var_rl_filename_quote_characters=yes],
247+
[pgac_cv_var_rl_filename_quote_characters=no])])
248+
if test x"$pgac_cv_var_rl_filename_quote_characters" = x"yes"; then
249+
AC_DEFINE(HAVE_RL_FILENAME_QUOTE_CHARACTERS, 1,
250+
[Define to 1 if you have the global variable 'rl_filename_quote_characters'.])
251+
fi
252+
AC_CACHE_CHECK([for rl_filename_quoting_function], pgac_cv_var_rl_filename_quoting_function,
253+
[AC_LINK_IFELSE([AC_LANG_PROGRAM([#include <stdio.h>
254+
#if defined(HAVE_READLINE_READLINE_H)
255+
#include <readline/readline.h>
256+
#elif defined(HAVE_EDITLINE_READLINE_H)
257+
#include <editline/readline.h>
258+
#elif defined(HAVE_READLINE_H)
259+
#include <readline.h>
260+
#endif
261+
],
262+
[rl_filename_quoting_function = 0;])],
263+
[pgac_cv_var_rl_filename_quoting_function=yes],
264+
[pgac_cv_var_rl_filename_quoting_function=no])])
265+
if test x"$pgac_cv_var_rl_filename_quoting_function" = x"yes"; then
266+
AC_DEFINE(HAVE_RL_FILENAME_QUOTING_FUNCTION, 1,
267+
[Define to 1 if you have the global variable 'rl_filename_quoting_function'.])
268+
fi
269+
])# PGAC_READLINE_VARIABLES
232270

233271

234272

‎configure‎

Lines changed: 84 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16316,10 +16316,12 @@ else
1631616316
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
1631716317
/* end confdefs.h. */
1631816318
#include <stdio.h>
16319-
#ifdef HAVE_READLINE_READLINE_H
16320-
# include <readline/readline.h>
16319+
#if defined(HAVE_READLINE_READLINE_H)
16320+
#include <readline/readline.h>
16321+
#elif defined(HAVE_EDITLINE_READLINE_H)
16322+
#include <editline/readline.h>
1632116323
#elif defined(HAVE_READLINE_H)
16322-
# include <readline.h>
16324+
#include <readline.h>
1632316325
#endif
1632416326

1632516327
int
@@ -16345,6 +16347,85 @@ if test x"$pgac_cv_var_rl_completion_append_character" = x"yes"; then
1634516347
$as_echo "#define HAVE_RL_COMPLETION_APPEND_CHARACTER 1" >>confdefs.h
1634616348

1634716349
fi
16350+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for rl_filename_quote_characters" >&5
16351+
$as_echo_n "checking for rl_filename_quote_characters... " >&6; }
16352+
if ${pgac_cv_var_rl_filename_quote_characters+:} false; then :
16353+
$as_echo_n "(cached) " >&6
16354+
else
16355+
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
16356+
/* end confdefs.h. */
16357+
#include <stdio.h>
16358+
#if defined(HAVE_READLINE_READLINE_H)
16359+
#include <readline/readline.h>
16360+
#elif defined(HAVE_EDITLINE_READLINE_H)
16361+
#include <editline/readline.h>
16362+
#elif defined(HAVE_READLINE_H)
16363+
#include <readline.h>
16364+
#endif
16365+
16366+
int
16367+
main ()
16368+
{
16369+
rl_filename_quote_characters = "x";
16370+
;
16371+
return 0;
16372+
}
16373+
_ACEOF
16374+
if ac_fn_c_try_link "$LINENO"; then :
16375+
pgac_cv_var_rl_filename_quote_characters=yes
16376+
else
16377+
pgac_cv_var_rl_filename_quote_characters=no
16378+
fi
16379+
rm -f core conftest.err conftest.$ac_objext \
16380+
conftest$ac_exeext conftest.$ac_ext
16381+
fi
16382+
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_var_rl_filename_quote_characters" >&5
16383+
$as_echo "$pgac_cv_var_rl_filename_quote_characters" >&6; }
16384+
if test x"$pgac_cv_var_rl_filename_quote_characters" = x"yes"; then
16385+
16386+
$as_echo "#define HAVE_RL_FILENAME_QUOTE_CHARACTERS 1" >>confdefs.h
16387+
16388+
fi
16389+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for rl_filename_quoting_function" >&5
16390+
$as_echo_n "checking for rl_filename_quoting_function... " >&6; }
16391+
if ${pgac_cv_var_rl_filename_quoting_function+:} false; then :
16392+
$as_echo_n "(cached) " >&6
16393+
else
16394+
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
16395+
/* end confdefs.h. */
16396+
#include <stdio.h>
16397+
#if defined(HAVE_READLINE_READLINE_H)
16398+
#include <readline/readline.h>
16399+
#elif defined(HAVE_EDITLINE_READLINE_H)
16400+
#include <editline/readline.h>
16401+
#elif defined(HAVE_READLINE_H)
16402+
#include <readline.h>
16403+
#endif
16404+
16405+
int
16406+
main ()
16407+
{
16408+
rl_filename_quoting_function = 0;
16409+
;
16410+
return 0;
16411+
}
16412+
_ACEOF
16413+
if ac_fn_c_try_link "$LINENO"; then :
16414+
pgac_cv_var_rl_filename_quoting_function=yes
16415+
else
16416+
pgac_cv_var_rl_filename_quoting_function=no
16417+
fi
16418+
rm -f core conftest.err conftest.$ac_objext \
16419+
conftest$ac_exeext conftest.$ac_ext
16420+
fi
16421+
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_var_rl_filename_quoting_function" >&5
16422+
$as_echo "$pgac_cv_var_rl_filename_quoting_function" >&6; }
16423+
if test x"$pgac_cv_var_rl_filename_quoting_function" = x"yes"; then
16424+
16425+
$as_echo "#define HAVE_RL_FILENAME_QUOTING_FUNCTION 1" >>confdefs.h
16426+
16427+
fi
16428+
1634816429
for ac_func in rl_completion_matches rl_filename_completion_function rl_reset_screen_size
1634916430
do :
1635016431
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`

‎configure.in‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1874,7 +1874,7 @@ fi
18741874
LIBS="$LIBS_including_readline"
18751875

18761876
if test "$with_readline" = yes; then
1877-
PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
1877+
PGAC_READLINE_VARIABLES
18781878
AC_CHECK_FUNCS([rl_completion_matches rl_filename_completion_function rl_reset_screen_size])
18791879
AC_CHECK_FUNCS([append_history history_truncate_file])
18801880
fi

‎src/bin/psql/stringutils.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,19 +282,21 @@ strip_quotes(char *source, char quote, char escape, int encoding)
282282
* entails_quote - any of these present? need outer quotes
283283
* quote - doubled within string, affixed to both ends
284284
* escape - doubled within string
285+
* force_quote - if true, quote the output even if it doesn't "need" it
285286
* encoding - the active character-set encoding
286287
*
287288
* Do not use this as a substitute for PQescapeStringConn(). Use it for
288289
* strings to be parsed by strtokx() or psql_scan_slash_option().
289290
*/
290291
char *
291292
quote_if_needed(const char *source, const char *entails_quote,
292-
char quote, char escape, int encoding)
293+
char quote, char escape, bool force_quote,
294+
int encoding)
293295
{
294296
const char *src;
295297
char *ret;
296298
char *dst;
297-
bool need_quotes = false;
299+
bool need_quotes = force_quote;
298300

299301
Assert(source != NULL);
300302
Assert(quote != '\0');

‎src/bin/psql/stringutils.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ extern char *strtokx(const char *s,
2222
extern void strip_quotes(char *source, char quote, char escape, int encoding);
2323

2424
extern char *quote_if_needed(const char *source, const char *entails_quote,
25-
char quote, char escape, int encoding);
25+
char quote, char escape, bool force_quote,
26+
int encoding);
2627

2728
#endif /* STRINGUTILS_H */

‎src/bin/psql/t/010_tab_completion.pl‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,30 @@
6060
# Some versions of readline inspect LS_COLORS, so for luck unset that too.
6161
delete $ENV{LS_COLORS};
6262

63+
# In a VPATH build, we'll be started in the source directory, but we want
64+
# to run in the build directory so that we can use relative paths to
65+
# access the tmp_check subdirectory; otherwise the output from filename
66+
# completion tests is too variable.
67+
if ($ENV{TESTDIR})
68+
{
69+
chdir $ENV{TESTDIR} or die "could not chdir to \"$ENV{TESTDIR}\": $!";
70+
}
71+
72+
# Create some junk files for filename completion testing.
73+
my $FH;
74+
open $FH, ">", "tmp_check/somefile"
75+
or die("could not create file \"tmp_check/somefile\": $!");
76+
print $FH "some stuff\n";
77+
close $FH;
78+
open $FH, ">", "tmp_check/afile123"
79+
or die("could not create file \"tmp_check/afile123\": $!");
80+
print $FH "more stuff\n";
81+
close $FH;
82+
open $FH, ">", "tmp_check/afile456"
83+
or die("could not create file \"tmp_check/afile456\": $!");
84+
print $FH "other stuff\n";
85+
close $FH;
86+
6387
# fire up an interactive psql session
6488
my $in = '';
6589
my $out = '';
@@ -104,6 +128,15 @@ sub clear_query
104128
return;
105129
}
106130

131+
# Clear current line to start over
132+
# (this will work in an incomplete string literal, but it's less desirable
133+
# than clear_query because we lose evidence in the history file)
134+
sub clear_line
135+
{
136+
check_completion("\025\n", qr/postgres=# /, "control-U works");
137+
return;
138+
}
139+
107140
# check basic command completion: SEL<tab> produces SELECT<space>
108141
check_completion("SEL\t", qr/SELECT /, "complete SEL<tab> to SELECT");
109142

@@ -142,6 +175,47 @@ sub clear_query
142175

143176
clear_query();
144177

178+
# check filename completion
179+
check_completion(
180+
"\\lo_import tmp_check/some\t",
181+
qr|tmp_check/somefile |,
182+
"filename completion with one possibility");
183+
184+
clear_query();
185+
186+
# note: readline might print a bell before the completion
187+
check_completion(
188+
"\\lo_import tmp_check/af\t",
189+
qr|tmp_check/af\a?ile|,
190+
"filename completion with multiple possibilities");
191+
192+
clear_query();
193+
194+
# COPY requires quoting
195+
# note: broken versions of libedit want to backslash the closing quote;
196+
# not much we can do about that
197+
check_completion(
198+
"COPY foo FROM tmp_check/some\t",
199+
qr|'tmp_check/somefile\\?' |,
200+
"quoted filename completion with one possibility");
201+
202+
clear_line();
203+
204+
check_completion(
205+
"COPY foo FROM tmp_check/af\t",
206+
< 8E05 span class="pl-pds">qr|'tmp_check/afile|,
207+
"quoted filename completion with multiple possibilities");
208+
209+
# some versions of readline/libedit require two tabs here, some only need one
210+
# also, some will offer the whole path name and some just the file name
211+
# the quotes might appear, too
212+
check_completion(
213+
"\t\t",
214+
qr|afile123'? +'?(tmp_check/)?afile456|,
215+
"offer multiple file choices");
216+
217+
clear_line();
218+
145219
# send psql an explicit \q to shut it down, else pty won't close properly
146220
$timer->start(5);
147221
$in .= "\\q\n";

0 commit comments

Comments
 (0)
0