Thread: psql prompts with invisible characters
Okay, Tom, here's take II.
warning: I've handchecked the sgml, but the doc tools I've got installed
die when I make man within doc/src/sgml. Specifically:
Unknown SDATA: [pi ] at
/usr/share/sgml/docbook/utils-0.6.12/helpers/docbook2man-spec.pl line
1240, <STDIN> line 108418.
I get this on an unmodified 7.4.1 tree as well, so I presume it's
because I've got an incompatible version of docbook2man-spec.pl . Tips?
-Reece
# add support for prompts with invisible characters to psql
#
# This patch adds support for readline prompts which contain non-printing
# characters as for colorized prompts or terminal title changes. This was
# nearly a direct lift from bash-2.05b's lib/readline/display.c, per
# guidance from Chet Ramey.
diff -ru --exclude='*.rej' --exclude='*.o' --exclude='*.orig' postgresql-7.4.1-orig/doc/src/sgml/ref/psql-ref.sgml
postgresql-7.4.1/doc/src/sgml/ref/psql-ref.sgml
--- postgresql-7.4.1-orig/doc/src/sgml/ref/psql-ref.sgml 2003-10-31 17:56:29.000000000 -0800
+++ postgresql-7.4.1/doc/src/sgml/ref/psql-ref.sgml 2004-01-16 09:56:54.000000000 -0800
@@ -2236,6 +2236,30 @@
</varlistentry>
<varlistentry>
+ <term><literal>%[</literal> ... <literal>%]</literal></term>
+ <listitem>
+ <para>
+ Prompts may contain terminal control characters which, for
+ example, change the foreground, background, or style of the prompt
+ text, or change the title of the terminal window. In order for
+ the line editing features of readline to work properly, these
+ non-printing control characters must be designated as invisible
+ by surrounding them with <literal>%[</literal> and
+ <literal>%]</literal>. Multiple pairs of these may occur within
+ the prompt. For example,
+<programlisting>
+testdb=> <userinput>\set PROMPT1 '%[%033[1;33;40m%]%n@%/%R%[%033[0m%#%] '</userinput>
+</programlisting>
+ results in a boldfaced (<literal>1;</literal>) yellow-on-black
+ <literal>33;40</literal> prompt for color-capable terminals.
+ See <ulink
+ url="http://www.termsys.demon.co.uk/vtansi.htm">http://www.termsys.demon.co.uk/vtansi.htm</a>
+ for an example of allowable codes.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>%R</literal></term>
<listitem>
<para>
diff -ru --exclude='*.rej' --exclude='*.o' --exclude='*.orig' postgresql-7.4.1-orig/src/bin/psql/prompt.c
postgresql-7.4.1/src/bin/psql/prompt.c
--- postgresql-7.4.1-orig/src/bin/psql/prompt.c 2003-10-03 18:04:46.000000000 -0700
+++ postgresql-7.4.1/src/bin/psql/prompt.c 2004-01-15 17:15:41.000000000 -0800
@@ -13,6 +13,7 @@
#include "settings.h"
#include "common.h"
#include "variables.h"
+#include "input.h"
#ifdef WIN32
#include <io.h>
@@ -241,6 +242,20 @@
buf[0] = '>';
break;
+ case '[':
+ case ']':
+#if defined (USE_READLINE)
+ buf[0] = '\001';
+ buf[1] = (*p == '[') ? RL_PROMPT_START_IGNORE : RL_PROMPT_END_IGNORE;
+ buf[2] = '\0';
+#else
+ /* Should we remove text between %[ and %]
+ e.g., for fancy prompts on dumb terminals ?
+ If so, under what conditions? */
+#endif /* USE_READLINE */
+ break;
+ /* end case [ or ] */
+
/* execute command */
case '`':
{
--
Reece Hart, http://www.in-machina.com/~reece/, GPG:0x25EC91A0
Reece Hart <reece@in-machina.com> writes:
> Okay, Tom, here's take II.
Okay, now that I understand what you want to do, how portable is this
across different readline versions --- how far back do they have support
for these RL_PROMPT_foo symbols, and do we need to consider the
possibility that the symbols aren't there or act differently?
BTW, you have a comment suggesting removing the contained text, but
that's all wrong --- lack of readline does not imply that the prompt is
being printed on a dumb terminal. We are leaving it up to the user to
select a prompt string that's compatible with his terminal; readline has
nothing to do with it. In short, ISTM that %[ and %] should simply be
ignored when readline isn't in use.
> warning: I've handchecked the sgml, but the doc tools I've got installed
> die when I make man within doc/src/sgml.
No idea about that. Maybe Peter will know.
regards, tom lane
On Fri, 2004-01-16 at 13:34, Tom Lane wrote:
Short answer: I believe that any readline newer than Feb 1999 oughta work.
The RL_PROMPT_* markers are undocumented features of readline. The patch I sent works fine with RL 4.2 and 4.3 on linux-x86. From the readline changelogs, I deduced that these were added with 4.0 (ca. Feb 1999). I recall that there have been fixes to the prompt display code over the years, but I believe these were not related to invisible char handling.
Obviously. Indeed that would be foolish logic. You and I are reading this differently and expounding on the several cases I was considering is immaterial since I wasn't advocating for stripping (i.e., protecting the user from himself) anyway. Let's chop the #else clause and leave it at that. I, too, am inclined to let the user shoot himself in the foot and get a garbled prompt if he uses a fancy prompt on an un-fancy term.
I see only three downsides to this patch:
1) In the case of someone who has %[ or %] in their prompt (I haven't a clue why they would) and expected [ or ] to get displayed, this patch will appear to be a behavior change.
2) Some users will undoubtedly be confused when they try a fancy prompt in a dumb term and see a prompt like
^[[1;33;40mrkh@csb=^[[0m>
(e.g., bring up psql in an emacs shell)
3) All features will generate pgsql- list traffic. This will be no different.
4) It relies on an undocumented feature in readline. Because bash uses this feature too and because they're both products of Chet Ramey, it's hard to see why this support would disappear (not impossible, mind you).
What else do you need to get this submitted? If it eases your mind, I'll wrap it in a --enable-prompt-invisibles configure flag so that it would be an experimental feature for the moment. I can only test this on linux-x86 and osf1-alpha... and I certainly have no idea what the windows or mac folks do (do they use configure at all?).
BTW, is there a coding style guide or pgsql-patch guide somewhere? For example, are patches acceptable as attachments (as opposed to inline)? Peter recently admonished someone for // comments, "personal" comments, and archive references... this is also good fodder for a style guide. And, is it preferred to diff against cvs instead? If so, which branch?
-Reece
how far back do they have support for these RL_PROMPT_foo symbols, and do we need to consider the possibility that the symbols aren't there or act differently?
Short answer: I believe that any readline newer than Feb 1999 oughta work.
The RL_PROMPT_* markers are undocumented features of readline. The patch I sent works fine with RL 4.2 and 4.3 on linux-x86. From the readline changelogs, I deduced that these were added with 4.0 (ca. Feb 1999). I recall that there have been fixes to the prompt display code over the years, but I believe these were not related to invisible char handling.
lack of readline does not imply that the prompt is being printed on a dumb terminal.
Obviously. Indeed that would be foolish logic. You and I are reading this differently and expounding on the several cases I was considering is immaterial since I wasn't advocating for stripping (i.e., protecting the user from himself) anyway. Let's chop the #else clause and leave it at that. I, too, am inclined to let the user shoot himself in the foot and get a garbled prompt if he uses a fancy prompt on an un-fancy term.
I see only three downsides to this patch:
1) In the case of someone who has %[ or %] in their prompt (I haven't a clue why they would) and expected [ or ] to get displayed, this patch will appear to be a behavior change.
2) Some users will undoubtedly be confused when they try a fancy prompt in a dumb term and see a prompt like
^[[1;33;40mrkh@csb=^[[0m>
(e.g., bring up psql in an emacs shell)
3) All features will generate pgsql- list traffic. This will be no different.
4) It relies on an undocumented feature in readline. Because bash uses this feature too and because they're both products of Chet Ramey, it's hard to see why this support would disappear (not impossible, mind you).
What else do you need to get this submitted? If it eases your mind, I'll wrap it in a --enable-prompt-invisibles configure flag so that it would be an experimental feature for the moment. I can only test this on linux-x86 and osf1-alpha... and I certainly have no idea what the windows or mac folks do (do they use configure at all?).
BTW, is there a coding style guide or pgsql-patch guide somewhere? For example, are patches acceptable as attachments (as opposed to inline)? Peter recently admonished someone for // comments, "personal" comments, and archive references... this is also good fodder for a style guide. And, is it preferred to diff against cvs instead? If so, which branch?
-Reece
-- Reece Hart, http://www.in-machina.com/~reece/, GPG:0x25EC91A0 0xD178AAF9 |
Reece Hart <reece@in-machina.com> writes:
> The RL_PROMPT_* markers are undocumented features of readline. The patch
> I sent works fine with RL 4.2 and 4.3 on linux-x86. From the readline
> changelogs, I deduced that these were added with 4.0 (ca. Feb 1999).
Hm. I don't recall whether we still pretend to support pre-4.0 readlines.
The "undocumented" bit actually bothers me rather more. I guess what we
can do is wrap the code in "ifdef RL_PROMPT_START_IGNORE" to keep from
blowing up if it's not present.
> Let's chop the #else clause and leave it at that.
Agreed.
> BTW, is there a coding style guide or pgsql-patch guide somewhere?
Not really; so far we've gotten away with "do like you see established
contributors doing".
> For
> example, are patches acceptable as attachments (as opposed to inline)?
Doesn't matter. We ask for "diff -c" format (plain diff is unsafe if
there have been any other changes in the files, and diff -u is harder to
read, at least in the opinions of those who are likely to review PG
patches). How you package it in your message is your choice. If you
use a mail program that might munge whitespace or linebreaks then an
attachment is probably the safest plan.
> And, is it preferred to diff against cvs instead? If so, which branch?
In general a diff against CVS HEAD will be the least pain to apply. In
this particular case it won't matter much, since those files haven't
changed recently.
regards, tom lane
Tom-
Here's take III of the patch, with the #ifdef for RL_PROMPT_START_IGNORE, the #else chopped, and made with diff -c against 7.4.1. A brief comment seemed warranted since this is an undocumented readline feature.
Note that the #if is:
#if defined (USE_READLINE) && defined (RL_PROMPT_START_IGNORE)
Although this is pedantic (R_P_S_I implies U_R), it seemed more transparent for those who come this way hence. Do as you like with it.
Fini?
-Reece
Here's take III of the patch, with the #ifdef for RL_PROMPT_START_IGNORE, the #else chopped, and made with diff -c against 7.4.1. A brief comment seemed warranted since this is an undocumented readline feature.
Note that the #if is:
#if defined (USE_READLINE) && defined (RL_PROMPT_START_IGNORE)
Although this is pedantic (R_P_S_I implies U_R), it seemed more transparent for those who come this way hence. Do as you like with it.
Fini?
-Reece
-- Reece Hart, http://www.in-machina.com/~reece/, GPG:0x25EC91A0 |
Attachment
Reece Hart <reece@in-machina.com> writes:
> Here's take III of the patch, with the #ifdef for
> RL_PROMPT_START_IGNORE, the #else chopped, and made with diff -c against
> 7.4.1. A brief comment seemed warranted since this is an undocumented
> readline feature.
Looks good, applied (with some minor editorialization on the docs).
regards, tom lane
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Reece Hart wrote:
> Okay, Tom, here's take II.
>
> warning: I've handchecked the sgml, but the doc tools I've got installed
> die when I make man within doc/src/sgml. Specifically:
>
> Unknown SDATA: [pi ] at
> /usr/share/sgml/docbook/utils-0.6.12/helpers/docbook2man-spec.pl line
> 1240, <STDIN> line 108418.
>
> I get this on an unmodified 7.4.1 tree as well, so I presume it's
> because I've got an incompatible version of docbook2man-spec.pl . Tips?
>
> -Reece
>
> # add support for prompts with invisible characters to psql
> #
> # This patch adds support for readline prompts which contain non-printing
> # characters as for colorized prompts or terminal title changes. This was
> # nearly a direct lift from bash-2.05b's lib/readline/display.c, per
> # guidance from Chet Ramey.
> diff -ru --exclude='*.rej' --exclude='*.o' --exclude='*.orig' postgresql-7.4.1-orig/doc/src/sgml/ref/psql-ref.sgml
postgresql-7.4.1/doc/src/sgml/ref/psql-ref.sgml
> --- postgresql-7.4.1-orig/doc/src/sgml/ref/psql-ref.sgml 2003-10-31 17:56:29.000000000 -0800
> +++ postgresql-7.4.1/doc/src/sgml/ref/psql-ref.sgml 2004-01-16 09:56:54.000000000 -0800
> @@ -2236,6 +2236,30 @@
> </varlistentry>
>
> <varlistentry>
> + <term><literal>%[</literal> ... <literal>%]</literal></term>
> + <listitem>
> + <para>
> + Prompts may contain terminal control characters which, for
> + example, change the foreground, background, or style of the prompt
> + text, or change the title of the terminal window. In order for
> + the line editing features of readline to work properly, these
> + non-printing control characters must be designated as invisible
> + by surrounding them with <literal>%[</literal> and
> + <literal>%]</literal>. Multiple pairs of these may occur within
> + the prompt. For example,
> +<programlisting>
> +testdb=> <userinput>\set PROMPT1 '%[%033[1;33;40m%]%n@%/%R%[%033[0m%#%] '</userinput>
> +</programlisting>
> + results in a boldfaced (<literal>1;</literal>) yellow-on-black
> + <literal>33;40</literal> prompt for color-capable terminals.
> + See <ulink
> + url="http://www.termsys.demon.co.uk/vtansi.htm">http://www.termsys.demon.co.uk/vtansi.htm</a>
> + for an example of allowable codes.
> + </para>
> + </listitem>
> + </varlistentry>
> +
> + <varlistentry>
> <term><literal>%R</literal></term>
> <listitem>
> <para>
> diff -ru --exclude='*.rej' --exclude='*.o' --exclude='*.orig' postgresql-7.4.1-orig/src/bin/psql/prompt.c
postgresql-7.4.1/src/bin/psql/prompt.c
> --- postgresql-7.4.1-orig/src/bin/psql/prompt.c 2003-10-03 18:04:46.000000000 -0700
> +++ postgresql-7.4.1/src/bin/psql/prompt.c 2004-01-15 17:15:41.000000000 -0800
> @@ -13,6 +13,7 @@
> #include "settings.h"
> #include "common.h"
> #include "variables.h"
> +#include "input.h"
>
> #ifdef WIN32
> #include <io.h>
> @@ -241,6 +242,20 @@
> buf[0] = '>';
> break;
>
> + case '[':
> + case ']':
> +#if defined (USE_READLINE)
> + buf[0] = '\001';
> + buf[1] = (*p == '[') ? RL_PROMPT_START_IGNORE : RL_PROMPT_END_IGNORE;
> + buf[2] = '\0';
> +#else
> + /* Should we remove text between %[ and %]
> + e.g., for fancy prompts on dumb terminals ?
> + If so, under what conditions? */
> +#endif /* USE_READLINE */
> + break;
> + /* end case [ or ] */
> +
> /* execute command */
> case '`':
> {
>
> --
> Reece Hart, http://www.in-machina.com/~reece/, GPG:0x25EC91A0
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
> joining column's datatypes do not match
>
--
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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Your patch has been added to the PostgreSQL unapplied patches list at:
Uh, wasn't this applied some time ago?
regards, tom lane
Sorry, already applied. Thanks. --------------------------------------------------------------------------- Reece Hart wrote: > Okay, Tom, here's take II. > > warning: I've handchecked the sgml, but the doc tools I've got installed > die when I make man within doc/src/sgml. Specifically: > > Unknown SDATA: [pi ] at > /usr/share/sgml/docbook/utils-0.6.12/helpers/docbook2man-spec.pl line > 1240, <STDIN> line 108418. > > I get this on an unmodified 7.4.1 tree as well, so I presume it's > because I've got an incompatible version of docbook2man-spec.pl . Tips? > > -Reece > > # add support for prompts with invisible characters to psql > # > # This patch adds support for readline prompts which contain non-printing > # characters as for colorized prompts or terminal title changes. This was > # nearly a direct lift from bash-2.05b's lib/readline/display.c, per > # guidance from Chet Ramey. > diff -ru --exclude='*.rej' --exclude='*.o' --exclude='*.orig' postgresql-7.4.1-orig/doc/src/sgml/ref/psql-ref.sgml postgresql-7.4.1/doc/src/sgml/ref/psql-ref.sgml > --- postgresql-7.4.1-orig/doc/src/sgml/ref/psql-ref.sgml 2003-10-31 17:56:29.000000000 -0800 > +++ postgresql-7.4.1/doc/src/sgml/ref/psql-ref.sgml 2004-01-16 09:56:54.000000000 -0800 > @@ -2236,6 +2236,30 @@ > </varlistentry> > > <varlistentry> > + <term><literal>%[</literal> ... <literal>%]</literal></term> > + <listitem> > + <para> > + Prompts may contain terminal control characters which, for > + example, change the foreground, background, or style of the prompt > + text, or change the title of the terminal window. In order for > + the line editing features of readline to work properly, these > + non-printing control characters must be designated as invisible > + by surrounding them with <literal>%[</literal> and > + <literal>%]</literal>. Multiple pairs of these may occur within > + the prompt. For example, > +<programlisting> > +testdb=> <userinput>\set PROMPT1 '%[%033[1;33;40m%]%n@%/%R%[%033[0m%#%] '</userinput> > +</programlisting> > + results in a boldfaced (<literal>1;</literal>) yellow-on-black > + <literal>33;40</literal> prompt for color-capable terminals. > + See <ulink > + url="http://www.termsys.demon.co.uk/vtansi.htm">http://www.termsys.demon.co.uk/vtansi.htm</a> > + for an example of allowable codes. > + </para> > + </listitem> > + </varlistentry> > + > + <varlistentry> > <term><literal>%R</literal></term> > <listitem> > <para> > diff -ru --exclude='*.rej' --exclude='*.o' --exclude='*.orig' postgresql-7.4.1-orig/src/bin/psql/prompt.c postgresql-7.4.1/src/bin/psql/prompt.c > --- postgresql-7.4.1-orig/src/bin/psql/prompt.c 2003-10-03 18:04:46.000000000 -0700 > +++ postgresql-7.4.1/src/bin/psql/prompt.c 2004-01-15 17:15:41.000000000 -0800 > @@ -13,6 +13,7 @@ > #include "settings.h" > #include "common.h" > #include "variables.h" > +#include "input.h" > > #ifdef WIN32 > #include <io.h> > @@ -241,6 +242,20 @@ > buf[0] = '>'; > break; > > + case '[': > + case ']': > +#if defined (USE_READLINE) > + buf[0] = '\001'; > + buf[1] = (*p == '[') ? RL_PROMPT_START_IGNORE : RL_PROMPT_END_IGNORE; > + buf[2] = '\0'; > +#else > + /* Should we remove text between %[ and %] > + e.g., for fancy prompts on dumb terminals ? > + If so, under what conditions? */ > +#endif /* USE_READLINE */ > + break; > + /* end case [ or ] */ > + > /* execute command */ > case '`': > { > > -- > Reece Hart, http://www.in-machina.com/~reece/, GPG:0x25EC91A0 > > > ---------------------------(end of broadcast)--------------------------- > TIP 9: the planner will ignore your desire to choose an index scan if your > joining column's datatypes do not match > -- 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
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Your patch has been added to the PostgreSQL unapplied patches list at: > > Uh, wasn't this applied some time ago? Yes, I see it now. I am not sure why I missed the commit message. It might have been during those few days where I had email problems. I checked all the lists but didn't check committers for previous patches. Thanks. -- 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