Re: TODO item -- Improve psql's handling of multi-line - Mailing list pgsql-patches

Modified patch attached and applied.  Thanks.

I adjusted based on Tom's comments to use a zero byte, and to clean up
the formatting.  I didn't see any extra non-readline overhead, just
calls to functions that are no-ops in non-readline cases.

---------------------------------------------------------------------------

Tom Lane wrote:
> "Sergey E. Koposov" <math@sai.msu.ru> writes:
> > On Wed, 7 Dec 2005, Andrew Dunstan wrote:
> >> A zero byte is probably a pretty bad choice. Some other low valued byte
> >> (e.g. \x01 ) would probably work better.
>
> > Currently I replace '\n' with the '\x01' as Andrew suggested.
>
> Won't this get confused by some of the Far Eastern encodings we support?
> The zero-byte approach is at least proof against that.  But what we need
> to ask is whether we can expect readline to cope with either.
>
> The patch *looks* pretty ugly: random insertions of blank space,
> general failure to conform to the project's code layout conventions,
> etc.  (Some of this would get cleaned up by pgindent, but I'm not sure
> how much.)  Also I get the impression that the patch enforces a lot of
> history maintenance overhead even in the non-USE_READLINE case, which is
> surely useless.
>
>             regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/bin/psql/help.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/help.c,v
retrieving revision 1.106
diff -c -c -r1.106 help.c
*** src/bin/psql/help.c    15 Oct 2005 02:49:40 -0000    1.106
--- src/bin/psql/help.c    11 Feb 2006 21:52:55 -0000
***************
*** 7,12 ****
--- 7,13 ----
   */
  #include "postgres_fe.h"
  #include "common.h"
+ #include "pqexpbuffer.h"
  #include "input.h"
  #include "print.h"
  #include "help.h"
Index: src/bin/psql/input.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/input.c,v
retrieving revision 1.46
diff -c -c -r1.46 input.c
*** src/bin/psql/input.c    15 Oct 2005 02:49:40 -0000    1.46
--- src/bin/psql/input.c    11 Feb 2006 21:52:56 -0000
***************
*** 7,14 ****
   */
  #include "postgres_fe.h"

- #include "input.h"
  #include "pqexpbuffer.h"
  #include "settings.h"
  #include "tab-complete.h"
  #include "common.h"
--- 7,14 ----
   */
  #include "postgres_fe.h"

  #include "pqexpbuffer.h"
+ #include "input.h"
  #include "settings.h"
  #include "tab-complete.h"
  #include "common.h"
***************
*** 90,107 ****
  #ifdef USE_READLINE
      char       *s;

-     static char *prev_hist = NULL;
-
      if (useReadline)
          /* On some platforms, readline is declared as readline(char *) */
          s = readline((char *) prompt);
      else
          s = gets_basic(prompt);

!     if (useHistory && s && s[0])
      {
!         enum histcontrol HC;

          HC = GetHistControlConfig();

          if (((HC & hctl_ignorespace) && s[0] == ' ') ||
--- 90,144 ----
  #ifdef USE_READLINE
      char       *s;

      if (useReadline)
          /* On some platforms, readline is declared as readline(char *) */
          s = readline((char *) prompt);
      else
          s = gets_basic(prompt);

!     return s;
! #else
!     return gets_basic(prompt);
! #endif
! }
!
!
! /* Put the line in the history buffer and also add the trailing \n */
! void pgadd_history(char *s, PQExpBuffer history_buf)
! {
! #ifdef USE_READLINE
!
!     int slen;
!     if (useReadline && useHistory && s && s[0])
      {
!         slen = strlen(s);
!         if (s[slen-1] == '\n')
!             appendPQExpBufferStr(history_buf, s);
!         else
!         {
!             appendPQExpBufferStr(history_buf, s);
!             appendPQExpBufferChar(history_buf, '\n');
!         }
!     }
! #endif
! }
!

+ /* Feed the contents of the history buffer to readline */
+ void pgflush_history(PQExpBuffer history_buf)
+ {
+ #ifdef USE_READLINE
+     char *s;
+     static char *prev_hist;
+     int slen, i;
+
+     if (useReadline && useHistory )
+     {
+         enum histcontrol HC;
+
+         s = history_buf->data;
+         prev_hist = NULL;
+
          HC = GetHistControlConfig();

          if (((HC & hctl_ignorespace) && s[0] == ' ') ||
***************
*** 112,128 ****
          else
          {
              free(prev_hist);
              prev_hist = pg_strdup(s);
              add_history(s);
          }
      }
-
-     return s;
- #else
-     return gets_basic(prompt);
  #endif
  }



  /*
--- 149,175 ----
          else
          {
              free(prev_hist);
+             slen = strlen(s);
+             /* Trim the trailing \n's */
+             for (i = slen-1; i >= 0 && s[i] == '\n'; i--)
+                 ;
+             s[i + 1] = '\0';
              prev_hist = pg_strdup(s);
              add_history(s);
          }
+
+         resetPQExpBuffer(history_buf);
      }
  #endif
  }

+ void pgclear_history(PQExpBuffer history_buf)
+ {
+ #ifdef USE_READLINE
+     if (useReadline && useHistory)
+         resetPQExpBuffer(history_buf);
+ #endif
+ }


  /*
***************
*** 157,162 ****
--- 204,233 ----
  }


+ static void encode_history()
+ {
+     HIST_ENTRY *cur_hist;
+     char *cur_ptr;
+
+     for (history_set_pos(0), cur_hist = current_history();
+          cur_hist; cur_hist = next_history())
+         for (cur_ptr = cur_hist->line; *cur_ptr; cur_ptr++)
+             if (*cur_ptr == '\n')
+                 *cur_ptr = '\0';
+ }
+
+ static void decode_history()
+ {
+     HIST_ENTRY *cur_hist;
+     char *cur_ptr;
+
+     for (history_set_pos(0), cur_hist = current_history();
+          cur_hist; cur_hist = next_history())
+         for (cur_ptr = cur_hist->line; *cur_ptr; cur_ptr++)
+             if (*cur_ptr == '\0')
+                 *cur_ptr = '\n';
+ }
+

  /*
   * Put any startup stuff related to input in here. It's good to maintain
***************
*** 197,202 ****
--- 268,275 ----

          if (psql_history)
              read_history(psql_history);
+
+         decode_history();
      }
  #endif

***************
*** 215,220 ****
--- 288,294 ----
  #ifdef USE_READLINE
      if (useHistory && fname)
      {
+         encode_history();
          if (write_history(fname) == 0)
              return true;

Index: src/bin/psql/input.h
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/input.h,v
retrieving revision 1.23
diff -c -c -r1.23 input.h
*** src/bin/psql/input.h    1 Jan 2005 05:43:08 -0000    1.23
--- src/bin/psql/input.h    11 Feb 2006 21:52:56 -0000
***************
*** 39,42 ****
--- 39,47 ----
  void        initializeInput(int flags);
  bool        saveHistory(char *fname);

+ void pgadd_history(char *s, PQExpBuffer history_buf);
+ void pgclear_history(PQExpBuffer history_buf);
+ void pgflush_history(PQExpBuffer history_buf);
+
+
  #endif   /* INPUT_H */
Index: src/bin/psql/mainloop.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v
retrieving revision 1.69
diff -c -c -r1.69 mainloop.c
*** src/bin/psql/mainloop.c    18 Dec 2005 02:17:16 -0000    1.69
--- src/bin/psql/mainloop.c    11 Feb 2006 21:52:56 -0000
***************
*** 37,42 ****
--- 37,43 ----
      PQExpBuffer query_buf;        /* buffer for query being accumulated */
      PQExpBuffer previous_buf;    /* if there isn't anything in the new buffer
                                   * yet, use this one for \e, etc. */
+     PQExpBuffer history_buf;
      char       *line;            /* current line of input */
      int            added_nl_pos;
      bool        success;
***************
*** 66,72 ****

      query_buf = createPQExpBuffer();
      previous_buf = createPQExpBuffer();
!     if (!query_buf || !previous_buf)
      {
          psql_error("out of memory\n");
          exit(EXIT_FAILURE);
--- 67,75 ----

      query_buf = createPQExpBuffer();
      previous_buf = createPQExpBuffer();
!     history_buf = createPQExpBuffer();
!
!     if (!query_buf || !previous_buf || !history_buf)
      {
          psql_error("out of memory\n");
          exit(EXIT_FAILURE);
***************
*** 90,96 ****
                  successResult = EXIT_USER;
                  break;
              }
!
              cancel_pressed = false;
          }

--- 93,99 ----
                  successResult = EXIT_USER;
                  break;
              }
!             pgclear_history(history_buf);
              cancel_pressed = false;
          }

***************
*** 106,111 ****
--- 109,116 ----
              count_eof = 0;
              slashCmdStatus = PSQL_CMD_UNKNOWN;
              prompt_status = PROMPT_READY;
+             if (pset.cur_cmd_interactive)
+                 pgclear_history(history_buf);

              if (pset.cur_cmd_interactive)
                  putc('\n', stdout);
***************
*** 138,148 ****
              psql_scan_reset(scan_state);
              slashCmdStatus = PSQL_CMD_UNKNOWN;
              prompt_status = PROMPT_READY;
          }
!
!         /*
!          * otherwise, get another line
!          */
          else if (pset.cur_cmd_interactive)
          {
              /* May need to reset prompt, eg after \r command */
--- 143,157 ----
              psql_scan_reset(scan_state);
              slashCmdStatus = PSQL_CMD_UNKNOWN;
              prompt_status = PROMPT_READY;
+
+             if (pset.cur_cmd_interactive)
+                 /*
+                  *    Pass all the contents of history_buf to readline
+                  *    and free the history buffer.
+                  */
+                 pgflush_history(history_buf);
          }
!         /* otherwise, get another line */
          else if (pset.cur_cmd_interactive)
          {
              /* May need to reset prompt, eg after \r command */
***************
*** 212,218 ****
           */
          psql_scan_setup(scan_state, line, strlen(line));
          success = true;
!
          while (success || !die_on_error)
          {
              PsqlScanResult scan_result;
--- 221,231 ----
           */
          psql_scan_setup(scan_state, line, strlen(line));
          success = true;
!
!         if (pset.cur_cmd_interactive)
!             /* Put current line in the history buffer */
!             pgadd_history(line, history_buf);
!
          while (success || !die_on_error)
          {
              PsqlScanResult scan_result;
***************
*** 287,292 ****
--- 300,312 ----
                  scan_result == PSCAN_EOL)
                  break;
          }
+
+         if (pset.cur_cmd_interactive && prompt_status != PROMPT_CONTINUE)
+             /*
+              *    Pass all the contents of history_buf to readline
+              *    and free the history buffer.
+              */
+             pgflush_history(history_buf);

          psql_scan_finish(scan_state);
          free(line);
***************
*** 333,338 ****
--- 353,359 ----

      destroyPQExpBuffer(query_buf);
      destroyPQExpBuffer(previous_buf);
+     destroyPQExpBuffer(history_buf);

      psql_scan_destroy(scan_state);

Index: src/bin/psql/prompt.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/prompt.c,v
retrieving revision 1.41
diff -c -c -r1.41 prompt.c
*** src/bin/psql/prompt.c    3 Jan 2006 23:32:30 -0000    1.41
--- src/bin/psql/prompt.c    11 Feb 2006 21:52:56 -0000
***************
*** 12,17 ****
--- 12,18 ----

  #include "settings.h"
  #include "common.h"
+ #include "pqexpbuffer.h"
  #include "input.h"
  #include "variables.h"

Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/tab-complete.c,v
retrieving revision 1.144
diff -c -c -r1.144 tab-complete.c
*** src/bin/psql/tab-complete.c    11 Jan 2006 08:43:12 -0000    1.144
--- src/bin/psql/tab-complete.c    11 Feb 2006 21:52:59 -0000
***************
*** 43,48 ****
--- 43,49 ----

  #include "postgres_fe.h"
  #include "tab-complete.h"
+ #include "pqexpbuffer.h"
  #include "input.h"

  /* If we don't have this, we might as well forget about the whole thing: */
***************
*** 50,56 ****

  #include <ctype.h>
  #include "libpq-fe.h"
- #include "pqexpbuffer.h"
  #include "common.h"
  #include "settings.h"

--- 51,56 ----