From fd66ccf7db638664a7867cc18e253d770c783d33 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 8 Sep 2014 16:09:49 -0400 Subject: [PATCH] Fix psql \s to work with recent libedit, and add pager support. psql's \s (print command history) doesn't work at all with recent libedit versions when printing to the terminal, because libedit tries to do an fchmod() on the target file which will fail if the target is /dev/tty. (We'd already noted this in the context of the target being /dev/null.) Even before that, it didn't work pleasantly, because libedit likes to encode the command history file (to ensure successful reloading), which renders it nigh unreadable, not to mention significantly different-looking depending on exactly which libedit version you have. So let's forget using write_history() for this purpose, and instead print the data ourselves, using logic similar to that used to iterate over the history for newline encoding/decoding purposes. While we're at it, insert the ability to use the pager when \s is printing to the terminal. This has been an acknowledged shortcoming of \s for many years, so while you could argue it's not exactly a back-patchable bug fix it still seems like a good improvement. Anyone who's seriously annoyed at this can use "\s /dev/tty" or local equivalent to get the old behavior. Experimentation with this showed that the history iteration logic was actually rather broken when used with libedit. It turns out that with libedit you have to use previous_history() not next_history() to advance to more recent history entries. The easiest and most robust fix for this seems to be to make a run-time test to verify which function to call. We had not noticed this because libedit doesn't really need the newline encoding logic: its own encoding ensures that command entries containing newlines are reloaded correctly (unlike libreadline). So the effective behavior with recent libedits was that only the oldest history entry got newline-encoded or newline-decoded. However, because of yet other bugs in history_set_pos(), some old versions of libedit allowed the existing loop logic to reach entries besides the oldest, which means there may be libedit ~/.psql_history files out there containing encoded newlines in more than just the oldest entry. To ensure we can reload such files, it seems appropriate to back-patch this fix, even though that will result in some incompatibility with older psql versions (ie, multiline history entries written by a psql with this fix will look corrupted to a psql without it, if its libedit is reasonably up to date). Stepan Rutz and Tom Lane --- doc/src/sgml/ref/psql-ref.sgml | 16 ++-- src/bin/psql/command.c | 14 +-- src/bin/psql/input.c | 163 ++++++++++++++++++++++++++------- src/bin/psql/input.h | 3 +- 4 files changed, 144 insertions(+), 52 deletions(-) diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index fd8661d7d0..6a57ad313f 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -265,7 +265,8 @@ EOF - Do not use readline for line editing and do not use the history. + Do not use Readline for line editing and do + not use the command history. This can be useful to turn off tab expansion when cutting and pasting. @@ -2344,12 +2345,13 @@ lo_import 152801 \s [ filename ] - Print or save the command line history to filename. If filename is omitted, the history - is written to the standard output. This option is only available - if psql is configured to use the - GNU Readline library. + Print psql's command line history + to filename. + If filename is omitted, + the history is written to the standard output (using the pager if + appropriate). This command is not available + if psql was built + without Readline support. diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index d18a7a6c35..0d9b677f1d 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -1088,20 +1088,8 @@ exec_command(const char *cmd, char *fname = psql_scan_slash_option(scan_state, OT_NORMAL, NULL, true); -#if defined(WIN32) && !defined(__CYGWIN__) - - /* - * XXX This does not work for all terminal environments or for output - * containing non-ASCII characters; see comments in simple_prompt(). - */ -#define DEVTTY "con" -#else -#define DEVTTY "/dev/tty" -#endif - expand_tilde(&fname); - /* This scrolls off the screen when using /dev/tty */ - success = saveHistory(fname ? fname : DEVTTY, -1, false, false); + success = printHistory(fname, pset.popt.topt.pager); if (success && !pset.quiet && fname) printf(_("Wrote history to file \"%s\".\n"), fname); if (!fname) diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c index aa32a3f5c1..6416ab91c2 100644 --- a/src/bin/psql/input.c +++ b/src/bin/psql/input.c @@ -11,6 +11,7 @@ #include #endif #include +#include #include "input.h" #include "settings.h" @@ -222,23 +223,73 @@ gets_fromFile(FILE *source) #ifdef USE_READLINE + +/* + * Macros to iterate over each element of the history list in order + * + * You would think this would be simple enough, but in its inimitable fashion + * libedit has managed to break it: in libreadline we must use next_history() + * to go from oldest to newest, but in libedit we must use previous_history(). + * To detect what to do, we make a trial call of previous_history(): if it + * fails, then either next_history() is what to use, or there's zero or one + * history entry so that it doesn't matter which direction we go. + * + * In case that wasn't disgusting enough: the code below is not as obvious as + * it might appear. In some libedit releases history_set_pos(0) fails until + * at least one add_history() call has been done. This is not an issue for + * printHistory() or encode_history(), which cannot be invoked before that has + * happened. In decode_history(), that's not so, and what actually happens is + * that we are sitting on the newest entry to start with, previous_history() + * fails, and we iterate over all the entries using next_history(). So the + * decode_history() loop iterates over the entries in the wrong order when + * using such a libedit release, and if there were another attempt to use + * BEGIN_ITERATE_HISTORY() before some add_history() call had happened, it + * wouldn't work. Fortunately we don't care about either of those things. + * + * Usage pattern is: + * + * BEGIN_ITERATE_HISTORY(varname); + * { + * loop body referencing varname->line; + * } + * END_ITERATE_HISTORY(); + */ +#define BEGIN_ITERATE_HISTORY(VARNAME) \ + do { \ + HIST_ENTRY *VARNAME; \ + bool use_prev_; \ + \ + history_set_pos(0); \ + use_prev_ = (previous_history() != NULL); \ + history_set_pos(0); \ + for (VARNAME = current_history(); VARNAME != NULL; \ + VARNAME = use_prev_ ? previous_history() : next_history()) \ + { \ + (void) 0 + +#define END_ITERATE_HISTORY() \ + } \ + } while(0) + + /* * Convert newlines to NL_IN_HISTORY for safe saving in readline history file */ static void encode_history(void) { - HIST_ENTRY *cur_hist; - char *cur_ptr; - - history_set_pos(0); - for (cur_hist = current_history(); cur_hist; cur_hist = next_history()) + BEGIN_ITERATE_HISTORY(cur_hist); { + char *cur_ptr; + /* some platforms declare HIST_ENTRY.line as const char * */ for (cur_ptr = (char *) cur_hist->line; *cur_ptr; cur_ptr++) + { if (*cur_ptr == '\n') *cur_ptr = NL_IN_HISTORY; + } } + END_ITERATE_HISTORY(); } /* @@ -247,17 +298,18 @@ encode_history(void) static void decode_history(void) { - HIST_ENTRY *cur_hist; - char *cur_ptr; - - history_set_pos(0); - for (cur_hist = current_history(); cur_hist; cur_hist = next_history()) + BEGIN_ITERATE_HISTORY(cur_hist); { + char *cur_ptr; + /* some platforms declare HIST_ENTRY.line as const char * */ for (cur_ptr = (char *) cur_hist->line; *cur_ptr; cur_ptr++) + { if (*cur_ptr == NL_IN_HISTORY) *cur_ptr = '\n'; + } } + END_ITERATE_HISTORY(); } #endif /* USE_READLINE */ @@ -319,22 +371,15 @@ initializeInput(int flags) /* - * This function saves the readline history when user - * runs \s command or when psql exits. + * This function saves the readline history when psql exits. * * fname: pathname of history file. (Should really be "const char *", * but some ancient versions of readline omit the const-decoration.) * * max_lines: if >= 0, limit history file to that many entries. - * - * appendFlag: if true, try to append just our new lines to the file. - * If false, write the whole available history. - * - * encodeFlag: whether to encode \n as \x01. For \s calls we don't wish - * to do that, but must do so when saving the final history file. */ -bool -saveHistory(char *fname, int max_lines, bool appendFlag, bool encodeFlag) +static bool +saveHistory(char *fname, int max_lines) { #ifdef USE_READLINE @@ -344,11 +389,15 @@ saveHistory(char *fname, int max_lines, bool appendFlag, bool encodeFlag) * where write_history will fail because it tries to chmod the target * file. */ - if (useHistory && fname && - strcmp(fname, DEVNULL) != 0) + if (strcmp(fname, DEVNULL) != 0) { - if (encodeFlag) - encode_history(); + /* + * Encode \n, since otherwise readline will reload multiline history + * entries as separate lines. (libedit doesn't really need this, but + * we do it anyway since it's too hard to tell which implementation we + * are using.) + */ + encode_history(); /* * On newer versions of libreadline, truncate the history file as @@ -362,7 +411,6 @@ saveHistory(char *fname, int max_lines, bool appendFlag, bool encodeFlag) * see if the write failed. Similarly for append_history. */ #if defined(HAVE_HISTORY_TRUNCATE_FILE) && defined(HAVE_APPEND_HISTORY) - if (appendFlag) { int nlines; int fd; @@ -387,8 +435,7 @@ saveHistory(char *fname, int max_lines, bool appendFlag, bool encodeFlag) if (errno == 0) return true; } - else -#endif +#else /* don't have append support */ { /* truncate what we have ... */ if (max_lines >= 0) @@ -399,19 +446,73 @@ saveHistory(char *fname, int max_lines, bool appendFlag, bool encodeFlag) if (errno == 0) return true; } +#endif psql_error("could not save history to file \"%s\": %s\n", fname, strerror(errno)); } -#else - /* only get here in \s case, so complain */ - psql_error("history is not supported by this installation\n"); #endif return false; } +/* + * Print history to the specified file, or to the console if fname is NULL + * (psql \s command) + * + * We used to use saveHistory() for this purpose, but that doesn't permit + * use of a pager; moreover libedit's implementation behaves incompatibly + * (preferring to encode its output) and may fail outright when the target + * file is specified as /dev/tty. + */ +bool +printHistory(const char *fname, unsigned short int pager) +{ +#ifdef USE_READLINE + FILE *output; + bool is_pager; + + if (!useHistory) + return false; + + if (fname == NULL) + { + /* use pager, if enabled, when printing to console */ + output = PageOutput(INT_MAX, pager); + is_pager = true; + } + else + { + output = fopen(fname, "w"); + if (output == NULL) + { + psql_error("could not save history to file \"%s\": %s\n", + fname, strerror(errno)); + return false; + } + is_pager = false; + } + + BEGIN_ITERATE_HISTORY(cur_hist); + { + fprintf(output, "%s\n", cur_hist->line); + } + END_ITERATE_HISTORY(); + + if (is_pager) + ClosePager(output); + else + fclose(output); + + return true; +#else + psql_error("history is not supported by this installation\n"); + return false; +#endif +} + + static void finishInput(void) { @@ -421,7 +522,7 @@ finishInput(void) int hist_size; hist_size = GetVariableNum(pset.vars, "HISTSIZE", 500, -1, true); - saveHistory(psql_history, hist_size, true, true); + (void) saveHistory(psql_history, hist_size); free(psql_history); psql_history = NULL; } diff --git a/src/bin/psql/input.h b/src/bin/psql/input.h index 1d10a22856..1b2239974a 100644 --- a/src/bin/psql/input.h +++ b/src/bin/psql/input.h @@ -42,7 +42,8 @@ char *gets_interactive(const char *prompt); char *gets_fromFile(FILE *source); void initializeInput(int flags); -bool saveHistory(char *fname, int max_lines, bool appendFlag, bool encodeFlag); + +bool printHistory(const char *fname, unsigned short int pager); void pg_append_history(const char *s, PQExpBuffer history_buf); void pg_send_history(PQExpBuffer history_buf); -- 2.40.0