Thread: fix of some issues with multi-line query editing
Fix of several issues: 1) Fix the problems with the \s command. When the saveHistory is executed by the \s command we must not do the conversion \n -> \x01 (per http://archives.postgresql.org/pgsql-hackers/2006-03/msg00317.php ) 2) Fix the handling of Ctrl+C Now when you do wsdb=# select 'your long query here ' wsdb-# and press afterwards the CtrlC the line "select 'your long query here '" will be in the history (partly per http://archives.postgresql.org/pgsql-hackers/2006-03/msg00297.php ) 3) Fix the handling of commands with not closed brackets, quotes, double quotes. (now those commands are not splitted in parts...) 4) Fix the behaviour when SINGLELINE mode is used. (before it was almost broken ;( Regards, Sergey ***************************************************** Sergey E. Koposov Max Planck Institute for Astronomy Web: http://lnfm1.sai.msu.ru/~math E-mail: math@sai.msu.ru
Attachment
I have applied the following patch which saves psql history for
backslash commands used in multi-line statements before the command,
rather than inside it, e.g:
test=> SELECT
test-> \d
No relations found.
test-> 1;
?column?
----------
1
(1 row)
has history in this order:
test=> \d
No relations found.
test=> SELECT
1;
?column?
----------
1
(1 row)
I also renamed some of the history functions for clarity.
---------------------------------------------------------------------------
Sergey E. Koposov wrote:
> Fix of several issues:
>
> 1) Fix the problems with the \s command.
> When the saveHistory is executed by the \s command we must not do the
> conversion \n -> \x01 (per
> http://archives.postgresql.org/pgsql-hackers/2006-03/msg00317.php )
>
> 2) Fix the handling of Ctrl+C
>
> Now when you do
> wsdb=# select 'your long query here '
> wsdb-#
> and press afterwards the CtrlC the line "select 'your long query here '"
> will be in the history
>
> (partly per
> http://archives.postgresql.org/pgsql-hackers/2006-03/msg00297.php )
>
> 3) Fix the handling of commands with not closed brackets, quotes, double
> quotes. (now those commands are not splitted in parts...)
>
> 4) Fix the behaviour when SINGLELINE mode is used. (before it was almost
> broken ;(
>
> Regards,
> Sergey
>
> *****************************************************
> Sergey E. Koposov
> Max Planck Institute for Astronomy
> Web: http://lnfm1.sai.msu.ru/~math
> E-mail: math@sai.msu.ru
>
>
>
Content-Description:
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
--
Bruce Momjian http://candle.pha.pa.us
SRA OSS, Inc. http://www.sraoss.com
+ If your life is a hard drive, Christ can be your backup. +
Index: src/bin/psql/input.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/input.c,v
retrieving revision 1.51
diff -c -c -r1.51 input.c
*** src/bin/psql/input.c 5 Mar 2006 15:58:51 -0000 1.51
--- src/bin/psql/input.c 6 Mar 2006 02:38:05 -0000
***************
*** 114,120 ****
/* Put the line in the history buffer and also add the trailing \n */
void
! pgadd_history(char *s, PQExpBuffer history_buf)
{
#ifdef USE_READLINE
--- 114,120 ----
/* Put the line in the history buffer and also add the trailing \n */
void
! pg_append_history(char *s, PQExpBuffer history_buf)
{
#ifdef USE_READLINE
***************
*** 134,145 ****
}
! /* 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;
--- 134,146 ----
}
! /*
! * Feed the string to readline
! */
void
! pg_write_history(char *s)
{
! #ifdef USE_READLINE
static char *prev_hist;
int slen, i;
***************
*** 147,153 ****
{
enum histcontrol HC;
- s = history_buf->data;
prev_hist = NULL;
HC = GetHistControlConfig();
--- 148,153 ----
***************
*** 168,181 ****
prev_hist = pg_strdup(s);
add_history(s);
}
-
- resetPQExpBuffer(history_buf);
}
#endif
}
void
! pgclear_history(PQExpBuffer history_buf)
{
#ifdef USE_READLINE
if (useReadline && useHistory)
--- 168,179 ----
prev_hist = pg_strdup(s);
add_history(s);
}
}
#endif
}
void
! pg_clear_history(PQExpBuffer history_buf)
{
#ifdef USE_READLINE
if (useReadline && useHistory)
Index: src/bin/psql/input.h
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/input.h,v
retrieving revision 1.25
diff -c -c -r1.25 input.h
*** src/bin/psql/input.h 5 Mar 2006 15:58:51 -0000 1.25
--- src/bin/psql/input.h 6 Mar 2006 02:38:05 -0000
***************
*** 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 */
--- 39,47 ----
void initializeInput(int flags);
bool saveHistory(char *fname);
! void pg_append_history(char *s, PQExpBuffer history_buf);
! void pg_clear_history(PQExpBuffer history_buf);
! void pg_write_history(char *s);
#endif /* INPUT_H */
Index: src/bin/psql/mainloop.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v
retrieving revision 1.71
diff -c -c -r1.71 mainloop.c
*** src/bin/psql/mainloop.c 5 Mar 2006 15:58:51 -0000 1.71
--- src/bin/psql/mainloop.c 6 Mar 2006 02:38:05 -0000
***************
*** 41,46 ****
--- 41,48 ----
char *line; /* current line of input */
int added_nl_pos;
bool success;
+ bool first_query_scan;
+
volatile int successResult = EXIT_SUCCESS;
volatile backslashResult slashCmdStatus = PSQL_CMD_UNKNOWN;
volatile promptStatus_t prompt_status = PROMPT_READY;
***************
*** 93,99 ****
successResult = EXIT_USER;
break;
}
! pgclear_history(history_buf);
cancel_pressed = false;
}
--- 95,101 ----
successResult = EXIT_USER;
break;
}
! pg_clear_history(history_buf);
cancel_pressed = false;
}
***************
*** 110,116 ****
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);
--- 112,118 ----
slashCmdStatus = PSQL_CMD_UNKNOWN;
prompt_status = PROMPT_READY;
if (pset.cur_cmd_interactive)
! pg_clear_history(history_buf);
if (pset.cur_cmd_interactive)
putc('\n', stdout);
***************
*** 145,155 ****
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)
--- 147,160 ----
prompt_status = PROMPT_READY;
if (pset.cur_cmd_interactive)
+ {
/*
* Pass all the contents of history_buf to readline
* and free the history buffer.
*/
! pg_write_history(history_buf->data);
! pg_clear_history(history_buf);
! }
}
/* otherwise, get another line */
else if (pset.cur_cmd_interactive)
***************
*** 221,230 ****
*/
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)
{
--- 226,232 ----
*/
psql_scan_setup(scan_state, line, strlen(line));
success = true;
! first_query_scan = true;
while (success || !die_on_error)
{
***************
*** 235,240 ****
--- 237,259 ----
prompt_status = prompt_tmp;
/*
+ * If we append to history a backslash command that is inside
+ * a multi-line query, then when we recall the history, the
+ * backslash command will make the query invalid, so we write
+ * backslash commands immediately rather than keeping them
+ * as part of the current multi-line query.
+ */
+ if (first_query_scan && pset.cur_cmd_interactive)
+ {
+ if (scan_result == PSCAN_BACKSLASH && query_buf->len != 0)
+ pg_write_history(line);
+ else
+ pg_append_history(line, history_buf);
+ }
+
+ first_query_scan = false;
+
+ /*
* Send command if semicolon found, or if end of line and we're in
* single-line mode.
*/
***************
*** 302,312 ****
}
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);
--- 321,334 ----
}
if (pset.cur_cmd_interactive && prompt_status != PROMPT_CONTINUE)
+ {
/*
* Pass all the contents of history_buf to readline
* and free the history buffer.
*/
! pg_write_history(history_buf->data);
! pg_clear_history(history_buf);
! }
psql_scan_finish(scan_state);
free(line);
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> 3) Fix the handling of commands with not closed brackets, quotes, double
>> quotes. (now those commands are not splitted in parts...)
Really? CVS tip seems to treat a multiline command as a single history
entry *unless* it contains newlines within quoted strings (including
single, double, dollar quotes) or newlines within /* comments.
I think this patch is seriously broken, and I don't agree with what it's
trying to accomplish in the first place --- I still haven't found any
cases where it's an improvement to pull back multiple lines as one
history entry. For example, if you made a mistake on the fifth line of
a ten-line CREATE TABLE command, the current code makes it extremely
inconvenient to fix that: you have to back-arrow or forward-arrow
tediously over five lines, where before you could pull back one line at
a time and just edir the line that needed fixing.
I'm still a vote to revert the patch altogether. But there is no theory
under which the current behavior is sane.
regards, tom lane
On Sun, 12 Mar 2006, Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> 3) Fix the handling of commands with not closed brackets, quotes, double
> >> quotes. (now those commands are not splitted in parts...)
>
> Really? CVS tip seems to treat a multiline command as a single history
> entry *unless* it contains newlines within quoted strings (including
> single, double, dollar quotes) or newlines within /* comments.
Bruce have NOT applied MY patch, which really contained the fixes of the
described problems... So, I'm waiting for it...
>
> I think this patch is seriously broken, and I don't agree with what it's
> trying to accomplish in the first place --- I still haven't found any
> cases where it's an improvement to pull back multiple lines as one
> history entry. For example, if you made a mistake on the fifth line of
> a ten-line CREATE TABLE command, the current code makes it extremely
> inconvenient to fix that: you have to back-arrow or forward-arrow
> tediously over five lines, where before you could pull back one line at
> a time and just edir the line that needed fixing.
(you do always Ctrl+Left_arrow ... to do that faster...) I don't see
anything bad here, that's the normal way to do it...
Regards,
Sergey
*****************************************************
Sergey E. Koposov
Max Planck Institute for Astronomy
Web: http://lnfm1.sai.msu.ru/~math
E-mail: math@sai.msu.ru
Sergey E. Koposov wrote: > > On Sun, 12 Mar 2006, Tom Lane wrote: > > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > >> 3) Fix the handling of commands with not closed brackets, quotes, double > > >> quotes. (now those commands are not splitted in parts...) > > > > Really? CVS tip seems to treat a multiline command as a single history > > entry *unless* it contains newlines within quoted strings (including > > single, double, dollar quotes) or newlines within /* comments. > > > Bruce have NOT applied MY patch, which really contained the fixes of the > described problems... So, I'm waiting for it... Uh, what patch? I don't have any additional patch for this? Can you resend it or give the subject line. -- Bruce Momjian http://candle.pha.pa.us SRA OSS, Inc. http://www.sraoss.com + If your life is a hard drive, Christ can be your backup. +
Tom Lane wrote: > I think this patch is seriously broken, and I don't agree with what it's > trying to accomplish in the first place --- I still haven't found any > cases where it's an improvement to pull back multiple lines as one > history entry. For example, if you made a mistake on the fifth line of > a ten-line CREATE TABLE command, the current code makes it extremely > inconvenient to fix that: you have to back-arrow or forward-arrow > tediously over five lines, where before you could pull back one line at > a time and just edir the line that needed fixing. Huh, I find precisely that behavior very difficult to use; you end up pressing up-arrow ten times to get the first line, ten times to get the second line, ten more times to get the third line, up to a total of a hundred times! And if you press it nine or eleven times instead of ten and press "enter" before realizing it (a mistake very easily made), you are hosed and have to start all over. Instead of waiting for back-arrow I usually use Alt-b and Alt-f. Or if I have to go back 20 words, esc-20 alt-b. This is much quicker. (I agree that there are still bugs. But we should correct those, not remove the useful behavior.) -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Mon, 13 Mar 2006, Bruce Momjian wrote: > Sergey E. Koposov wrote: > > Bruce have NOT applied MY patch, which really contained the fixes of the > > described problems... So, I'm waiting for it... > > Uh, what patch? I don't have any additional patch for this? Can you > resend it or give the subject line. > The patch from here: http://archives.postgresql.org/pgsql-patches/2006-03/msg00058.php (from exactly This thread: [fix of some issues with multi-line query editing]) ^^^^ Regards, Sergey ***************************************************** Sergey E. Koposov Max Planck Institute for Astronomy Web: http://lnfm1.sai.msu.ru/~math E-mail: math@sai.msu.ru
Sergey E. Koposov wrote: > On Mon, 13 Mar 2006, Bruce Momjian wrote: > > Sergey E. Koposov wrote: > > > Bruce have NOT applied MY patch, which really contained the fixes of the > > > described problems... So, I'm waiting for it... > > > > Uh, what patch? I don't have any additional patch for this? Can you > > resend it or give the subject line. > > > > The patch from here: > http://archives.postgresql.org/pgsql-patches/2006-03/msg00058.php > > (from exactly This thread: [fix of some issues with multi-line query editing]) Great. Sorry I had missed that patch. Added to patch queue now. -- Bruce Momjian http://candle.pha.pa.us SRA OSS, Inc. http://www.sraoss.com + If your life is a hard drive, Christ can be your backup. +
Sergey E. Koposov wrote: > Fix of several issues: > > 1) Fix the problems with the \s command. > When the saveHistory is executed by the \s command we must not do the > conversion \n -> \x01 (per > http://archives.postgresql.org/pgsql-hackers/2006-03/msg00317.php ) > > 2) Fix the handling of Ctrl+C > > Now when you do > wsdb=# select 'your long query here ' > wsdb-# > and press afterwards the CtrlC the line "select 'your long query here '" > will be in the history > > (partly per > http://archives.postgresql.org/pgsql-hackers/2006-03/msg00297.php ) > > 3) Fix the handling of commands with not closed brackets, quotes, double > quotes. (now those commands are not splitted in parts...) > > 4) Fix the behaviour when SINGLELINE mode is used. (before it was almost > broken ;( Great. Patch applied. I had to adjust your patch around changes made to put backslash commands embedded in queries to the top of the history. Updated patch attached. -- Bruce Momjian http://candle.pha.pa.us SRA OSS, Inc. http://www.sraoss.com + If your life is a hard drive, Christ can be your backup. + Index: src/bin/psql/command.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v retrieving revision 1.164 diff -c -c -r1.164 command.c *** src/bin/psql/command.c 5 Mar 2006 15:58:51 -0000 1.164 --- src/bin/psql/command.c 21 Mar 2006 12:50:34 -0000 *************** *** 753,759 **** expand_tilde(&fname); /* This scrolls off the screen when using /dev/tty */ ! success = saveHistory(fname ? fname : DEVTTY); if (success && !quiet && fname) printf(gettext("Wrote history to file \"%s/%s\".\n"), pset.dirname ? pset.dirname : ".", fname); --- 753,759 ---- expand_tilde(&fname); /* This scrolls off the screen when using /dev/tty */ ! success = saveHistory(fname ? fname : DEVTTY, false); if (success && !quiet && fname) printf(gettext("Wrote history to file \"%s/%s\".\n"), pset.dirname ? pset.dirname : ".", fname); Index: src/bin/psql/input.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/psql/input.c,v retrieving revision 1.52 diff -c -c -r1.52 input.c *** src/bin/psql/input.c 6 Mar 2006 04:45:21 -0000 1.52 --- src/bin/psql/input.c 21 Mar 2006 12:50:34 -0000 *************** *** 148,153 **** --- 148,157 ---- { enum histcontrol HC; + /* Flushing of empty buffer should do nothing */ + if (*s == 0) + return; + prev_hist = NULL; HC = GetHistControlConfig(); *************** *** 295,307 **** } bool ! saveHistory(char *fname) { #ifdef USE_READLINE if (useHistory && fname) { ! encode_history(); if (write_history(fname) == 0) return true; --- 299,318 ---- } + /* This function is designed for saving the readline history when user + * run \s command or when psql finishes. + * We have an argument named encodeFlag to handle those cases differently + * In that case of call via \s we don't really need to encode \n as \x01, + * but when we save history for Readline we must do that conversion + */ bool ! saveHistory(char *fname, bool encodeFlag) { #ifdef USE_READLINE if (useHistory && fname) { ! if (encodeFlag) ! encode_history(); if (write_history(fname) == 0) return true; *************** *** 331,337 **** if (hist_size >= 0) stifle_history(hist_size); ! saveHistory(psql_history); free(psql_history); psql_history = NULL; } --- 342,348 ---- if (hist_size >= 0) stifle_history(hist_size); ! saveHistory(psql_history, true); free(psql_history); psql_history = NULL; } Index: src/bin/psql/input.h =================================================================== RCS file: /cvsroot/pgsql/src/bin/psql/input.h,v retrieving revision 1.26 diff -c -c -r1.26 input.h *** src/bin/psql/input.h 6 Mar 2006 04:45:21 -0000 1.26 --- src/bin/psql/input.h 21 Mar 2006 12:50:34 -0000 *************** *** 37,43 **** char *gets_fromFile(FILE *source); void initializeInput(int flags); ! bool saveHistory(char *fname); void pg_append_history(char *s, PQExpBuffer history_buf); void pg_clear_history(PQExpBuffer history_buf); --- 37,43 ---- char *gets_fromFile(FILE *source); void initializeInput(int flags); ! bool saveHistory(char *fname, bool encodeFlag); void pg_append_history(char *s, PQExpBuffer history_buf); void pg_clear_history(PQExpBuffer history_buf); Index: src/bin/psql/mainloop.c =================================================================== RCS file: /cvsroot/pgsql/src/bin/psql/mainloop.c,v retrieving revision 1.73 diff -c -c -r1.73 mainloop.c *** src/bin/psql/mainloop.c 6 Mar 2006 15:09:04 -0000 1.73 --- src/bin/psql/mainloop.c 21 Mar 2006 12:50:34 -0000 *************** *** 112,118 **** slashCmdStatus = PSQL_CMD_UNKNOWN; prompt_status = PROMPT_READY; if (pset.cur_cmd_interactive) ! pg_clear_history(history_buf); if (pset.cur_cmd_interactive) putc('\n', stdout); --- 112,118 ---- slashCmdStatus = PSQL_CMD_UNKNOWN; prompt_status = PROMPT_READY; if (pset.cur_cmd_interactive) ! pg_write_history(history_buf->data); if (pset.cur_cmd_interactive) putc('\n', stdout); *************** *** 321,327 **** break; } ! if (pset.cur_cmd_interactive && prompt_status != PROMPT_CONTINUE) { /* * Pass all the contents of history_buf to readline --- 321,328 ---- break; } ! if ((pset.cur_cmd_interactive && prompt_status == PROMPT_READY) || ! (GetVariableBool(pset.vars, "SINGLELINE") && prompt_status == PROMPT_CONTINUE)) { /* * Pass all the contents of history_buf to readline
Bruce Momjian wrote: > Great. Patch applied. I had to adjust your patch around changes made > to put backslash commands embedded in queries to the top of the history. I have tested it and it works according to my expectations; the bugs I noticed are no longer there. Thanks! It's a bit annoying (you could say bogus) when the query buffer is longer than the terminal. Not sure if that's a problem of the patch or it has always been like this. It's possible to workaround using \e. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support