I wrote:
> What I'm inclined to do based on this info is to start the loop at
> history_base - 1, and ignore NULL returns until we're past history_base.
I poked at that for awhile and decided it was a bad approach. It emerges
that libedit's history_get() is just as full of version-specific
misbehaviors as the next_history() approach. While we could possibly
work around all those bugs, it's true in all libedit versions that
history_get(N) is O(N) because it iterates down the history list. So
looping over the whole history list with it is O(N^2) in the number of
history entries, which could well get painful with long history lists.
What seems better is to stick with the history_set_pos/next_history
approach, but adapt it to use previous_history when required. This is
O(N) overall in both libreadline and libedit. I therefore propose the
attached patch.
Experimenting with this, it seems to work as expected in Apple's
libedit-13 and up (corresponding to OS X Snow Leopard and newer). The
fact that it works in pre-Mavericks releases is a bit accidental, because
history_set_pos() is in fact partially broken in those releases, per
comments in the patch. And it doesn't work very well in libedit-5.1 (OS X
Tiger) because history_set_pos() is seemingly *completely* broken in that
release: it never succeeds, and we end up iterating over a subset of the
history list that does not seem to have any rhyme or reason to it.
However I don't think that this patch makes things any worse than they
were before with that release.
I only tried this directly on Tiger, Snow Leopard, and Mavericks. I
tested libedit-28 by compiling from source on a RHEL machine, so it's
possible that there's some difference between what I tested and what
Apple's really shipping. If anyone wants to try it on other platforms,
feel free.
[ wanders away wondering how it is that libedit has any following
whatsoever ... ]
regards, tom lane
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a66093a..39b5777 100644
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*************** exec_command(const char *cmd,
*** 1088,1107 ****
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);
if (success && !pset.quiet && fname)
printf(_("Wrote history to file \"%s\".\n"), fname);
if (!fname)
--- 1088,1095 ----
char *fname = psql_scan_slash_option(scan_state,
OT_NORMAL, NULL, true);
expand_tilde(&fname);
! 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 aa32a3f..136767f 100644
*** a/src/bin/psql/input.c
--- b/src/bin/psql/input.c
***************
*** 11,16 ****
--- 11,17 ----
#include <unistd.h>
#endif
#include <fcntl.h>
+ #include <limits.h>
#include "input.h"
#include "settings.h"
*************** gets_fromFile(FILE *source)
*** 222,244 ****
#ifdef USE_READLINE
/*
* 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())
{
/* 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;
}
}
/*
--- 223,285 ----
#ifdef USE_READLINE
+
+ /*
+ * Macros to iterate over each element of the history list
+ *
+ * You would think this would be simple enough, but in its inimitable fashion
+ * libedit has managed to break it: in all but very old libedit releases it is
+ * necessary to iterate using previous_history(), whereas in libreadline the
+ * call to use is next_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.
+ *
+ * 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.
+ */
+ #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)
{
! 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();
}
/*
*************** encode_history(void)
*** 247,263 ****
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())
{
/* 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';
}
}
#endif /* USE_READLINE */
--- 288,305 ----
static void
decode_history(void)
{
! 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 */
*************** initializeInput(int flags)
*** 319,340 ****
/*
! * This function saves the readline history when user
! * runs \s command or 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)
{
#ifdef USE_READLINE
--- 361,375 ----
/*
! * 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.
*/
! static bool
! saveHistory(char *fname, int max_lines)
{
#ifdef USE_READLINE
*************** saveHistory(char *fname, int max_lines,
*** 344,354 ****
* where write_history will fail because it tries to chmod the target
* file.
*/
! if (useHistory && fname &&
! strcmp(fname, DEVNULL) != 0)
{
! if (encodeFlag)
! encode_history();
/*
* On newer versions of libreadline, truncate the history file as
--- 379,393 ----
* where write_history will fail because it tries to chmod the target
* file.
*/
! if (strcmp(fname, DEVNULL) != 0)
{
! /*
! * 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
*************** saveHistory(char *fname, int max_lines,
*** 362,368 ****
* 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;
--- 401,406 ----
*************** saveHistory(char *fname, int max_lines,
*** 387,394 ****
if (errno == 0)
return true;
}
! else
! #endif
{
/* truncate what we have ... */
if (max_lines >= 0)
--- 425,431 ----
if (errno == 0)
return true;
}
! #else /* don't have append support */
{
/* truncate what we have ... */
if (max_lines >= 0)
*************** saveHistory(char *fname, int max_lines,
*** 399,417 ****
if (errno == 0)
return true;
}
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;
}
static void
finishInput(void)
{
--- 436,508 ----
if (errno == 0)
return true;
}
+ #endif
psql_error("could not save history to file \"%s\": %s\n",
fname, strerror(errno));
}
#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)
{
*************** finishInput(void)
*** 421,427 ****
int hist_size;
hist_size = GetVariableNum(pset.vars, "HISTSIZE", 500, -1, true);
! saveHistory(psql_history, hist_size, true, true);
free(psql_history);
psql_history = NULL;
}
--- 512,518 ----
int hist_size;
hist_size = GetVariableNum(pset.vars, "HISTSIZE", 500, -1, 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 1d10a22..1b22399 100644
*** a/src/bin/psql/input.h
--- b/src/bin/psql/input.h
*************** char *gets_interactive(const char *pr
*** 42,48 ****
char *gets_fromFile(FILE *source);
void initializeInput(int flags);
! bool saveHistory(char *fname, int max_lines, bool appendFlag, bool encodeFlag);
void pg_append_history(const char *s, PQExpBuffer history_buf);
void pg_send_history(PQExpBuffer history_buf);
--- 42,49 ----
char *gets_fromFile(FILE *source);
void initializeInput(int flags);
!
! 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);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
