Horton

Thread

Commits

  1. psql: Fix %w length in PROMPT2 when PROMPT1 contains a newline.

  2. Allow invisible PROMPT2 in psql.

  1. Invisible PROMPT2

    Thomas Munro <thomas.munro@gmail.com> — 2019-11-13T03:14:44Z

    Hello hackers,

From the advanced bikeshedding department: I'd like my psql
transcripts to have the usual alignment, but be easier to copy and
paste later without having weird prompt stuff in the middle.  How
about a prompt format directive %w that means "whitespace of the same
width as %/"?  Then you can make set your PROMPT2 to '%w   ' and it
becomes invisible:

pgdu=# create table foo (
         i int,
         j int
       );
CREATE TABLE
pgdu=#

  2. Re: Invisible PROMPT2

    Pavel Stehule <pavel.stehule@gmail.com> — 2019-11-13T05:58:11Z

    st 13. 11. 2019 v 4:15 odesílatel Thomas Munro <thomas.munro@gmail.com>
napsal:

> Hello hackers,
>
> From the advanced bikeshedding department: I'd like my psql
> transcripts to have the usual alignment, but be easier to copy and
> paste later without having weird prompt stuff in the middle.  How
> about a prompt format directive %w that means "whitespace of the same
> width as %/"?  Then you can make set your PROMPT2 to '%w   ' and it
> becomes invisible:
>
> pgdu=# create table foo (
>          i int,
>          j int
>        );
> CREATE TABLE
> pgdu=#
>

+1

Pavel

  3. Re: Invisible PROMPT2

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2019-11-13T11:27:00Z

    Thomas Munro <thomas.munro@gmail.com> writes:

> Hello hackers,
>
> From the advanced bikeshedding department: I'd like my psql
> transcripts to have the usual alignment, but be easier to copy and
> paste later without having weird prompt stuff in the middle.  How
> about a prompt format directive %w that means "whitespace of the same
> width as %/"?  Then you can make set your PROMPT2 to '%w   ' and it
> becomes invisible:

That only lines up nicely if %/ is the only variable-width directive in
PROMPT1.  How about a circumfix directive (like the existing %[ ... %])
that replaces everything inside with whitespace, but keeps the width?

- ilmari
-- 
"The surreality of the universe tends towards a maximum" -- Skud's Law
"Never formulate a law or axiom that you're not prepared to live with
 the consequences of."                              -- Skud's Meta-Law

  4. Re: Invisible PROMPT2

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-13T14:47:01Z

    ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> Thomas Munro <thomas.munro@gmail.com> writes:
>> From the advanced bikeshedding department: I'd like my psql
>> transcripts to have the usual alignment, but be easier to copy and
>> paste later without having weird prompt stuff in the middle.  How
>> about a prompt format directive %w that means "whitespace of the same
>> width as %/"?  Then you can make set your PROMPT2 to '%w   ' and it
>> becomes invisible:

> That only lines up nicely if %/ is the only variable-width directive in
> PROMPT1.

Yeah, that was my first reaction too.

> How about a circumfix directive (like the existing %[ ... %])
> that replaces everything inside with whitespace, but keeps the width?

Or just define %w as meaning "whitespace of the same width as
PROMPT1".  You couldn't use it *in* PROMPT1, then, but I see
no use-case for that anyway.

			regards, tom lane

  5. Re: Invisible PROMPT2

    David Fetter <david@fetter.org> — 2019-11-13T17:49:20Z

    On Wed, Nov 13, 2019 at 09:47:01AM -0500, Tom Lane wrote:
> ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:
> > Thomas Munro <thomas.munro@gmail.com> writes:
> >> From the advanced bikeshedding department: I'd like my psql
> >> transcripts to have the usual alignment, but be easier to copy and
> >> paste later without having weird prompt stuff in the middle.  How
> >> about a prompt format directive %w that means "whitespace of the same
> >> width as %/"?  Then you can make set your PROMPT2 to '%w   ' and it
> >> becomes invisible:
> 
> > That only lines up nicely if %/ is the only variable-width directive in
> > PROMPT1.
> 
> Yeah, that was my first reaction too.
> 
> > How about a circumfix directive (like the existing %[ ... %])
> > that replaces everything inside with whitespace, but keeps the width?
> 
> Or just define %w as meaning "whitespace of the same width as
> PROMPT1".  You couldn't use it *in* PROMPT1, then, but I see
> no use-case for that anyway.

+1 for doing it this way.  Would it make more sense to error out if
somebody tried to set that in PROMPT1, or ignore it, or...?

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  6. Re: Invisible PROMPT2

    Chapman Flack <chap@anastigmatix.net> — 2019-11-13T18:03:08Z

    On 11/13/19 12:49 PM, David Fetter wrote:
>> Or just define %w as meaning "whitespace of the same width as
>> PROMPT1".  You couldn't use it *in* PROMPT1, then, but I see
>> no use-case for that anyway.
> 
> +1 for doing it this way.  Would it make more sense to error out if
> somebody tried to set that in PROMPT1, or ignore it, or...?

Define it as "difference between PROMPT1's width and the total width
of non-%w elements in this prompt". Then it has a defined meaning in
PROMPT1 too (which could be arbitrary if it appears only once, but
has to be zero in case it appears more than once).

Easter egg: expand it to backspaces if used in PROMPT2 among other
stuff that's already wider than PROMPT1. ;)

Regards,
-Chap

  7. Re: Invisible PROMPT2

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-11-13T18:06:08Z

    On 2019-Nov-13, David Fetter wrote:

> On Wed, Nov 13, 2019 at 09:47:01AM -0500, Tom Lane wrote:

> > > How about a circumfix directive (like the existing %[ ... %])
> > > that replaces everything inside with whitespace, but keeps the width?
> > 
> > Or just define %w as meaning "whitespace of the same width as
> > PROMPT1".  You couldn't use it *in* PROMPT1, then, but I see
> > no use-case for that anyway.
> 
> +1 for doing it this way.  Would it make more sense to error out if
> somebody tried to set that in PROMPT1, or ignore it, or...?

This seems way too specific to me.  I like the "circumfix" directive
better, because it allows one to do more things.  I don't have any
immediate use for it, but it doesn't seem completely far-fetched that
there are some.

BTW the psql manual says that %[ and %] were plagiarized from tcsh, but
that's a lie: tcsh does not contain such a feature.  Bash does, however.
(I guess not many people read the tcsh manual.)

Neither bash nor tcsh have a feature to return whitespace of anything;
we're in a green field here ISTM.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  8. Re: Invisible PROMPT2

    David Fetter <david@fetter.org> — 2019-11-13T18:12:16Z

    On Wed, Nov 13, 2019 at 03:06:08PM -0300, Alvaro Herrera wrote:
> On 2019-Nov-13, David Fetter wrote:
> 
> > On Wed, Nov 13, 2019 at 09:47:01AM -0500, Tom Lane wrote:
> 
> > > > How about a circumfix directive (like the existing %[ ... %])
> > > > that replaces everything inside with whitespace, but keeps the width?
> > > 
> > > Or just define %w as meaning "whitespace of the same width as
> > > PROMPT1".  You couldn't use it *in* PROMPT1, then, but I see
> > > no use-case for that anyway.
> > 
> > +1 for doing it this way.  Would it make more sense to error out if
> > somebody tried to set that in PROMPT1, or ignore it, or...?
> 
> This seems way too specific to me.  I like the "circumfix" directive
> better, because it allows one to do more things.  I don't have any
> immediate use for it, but it doesn't seem completely far-fetched that
> there are some.
> 
> BTW the psql manual says that %[ and %] were plagiarized from tcsh, but
> that's a lie: tcsh does not contain such a feature.  Bash does, however.
> (I guess not many people read the tcsh manual.)
> 
> Neither bash nor tcsh have a feature to return whitespace of anything;
> we're in a green field here ISTM.

So something like %w[...%w] where people could put things like PROMPT1
inside?

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  9. Re: Invisible PROMPT2

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-11-13T18:58:38Z

    On 2019-Nov-13, David Fetter wrote:

> On Wed, Nov 13, 2019 at 03:06:08PM -0300, Alvaro Herrera wrote:
> > On 2019-Nov-13, David Fetter wrote:
> > 
> > > On Wed, Nov 13, 2019 at 09:47:01AM -0500, Tom Lane wrote:
> > 
> > > > > How about a circumfix directive (like the existing %[ ... %])
> > > > > that replaces everything inside with whitespace, but keeps the width?

> > This seems way too specific to me.  I like the "circumfix" directive
> > better, because it allows one to do more things.  I don't have any
> > immediate use for it, but it doesn't seem completely far-fetched that
> > there are some.

> So something like %w[...%w] where people could put things like PROMPT1
> inside?

Hmm, (I'm not sure your proposed syntax works, but let's assume that
it does.)  I'm saying you'd define
\set PROMPT1 '%a%b%c '
\set PROMPT2 '%w[%a%b%c %w]'

and you'd end up with matching indentation on multiline queries.

I'm not sure that we'd need to make something like this work:
  PROMPT1="%w[$PROMPT1%w]"
which I think is what you're saying.


We already have "%:PROMPT1:" but that expands to the literal value of
prompt1, not to the value that prompt1 would expand to:

55432 13devel 11214=# \set PROMPT2 'hello %:PROMPT1: bye'
55432 13devel 11214=# select<Enter>
hello %[%033[35m%]%> %:VERSION_NAME: %p%[%033[0m%]%R%#  bye

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  10. Re: Invisible PROMPT2

    David Fetter <david@fetter.org> — 2019-11-13T19:57:04Z

    On Wed, Nov 13, 2019 at 03:58:38PM -0300, Alvaro Herrera wrote:
> On 2019-Nov-13, David Fetter wrote:
> 
> > On Wed, Nov 13, 2019 at 03:06:08PM -0300, Alvaro Herrera wrote:
> > > On 2019-Nov-13, David Fetter wrote:
> > > 
> > > > On Wed, Nov 13, 2019 at 09:47:01AM -0500, Tom Lane wrote:
> > > 
> > > > > > How about a circumfix directive (like the existing %[ ... %])
> > > > > > that replaces everything inside with whitespace, but keeps the width?
> 
> > > This seems way too specific to me.  I like the "circumfix" directive
> > > better, because it allows one to do more things.  I don't have any
> > > immediate use for it, but it doesn't seem completely far-fetched that
> > > there are some.
> 
> > So something like %w[...%w] where people could put things like PROMPT1
> > inside?
> 
> Hmm, (I'm not sure your proposed syntax works, but let's assume that
> it does.)  I'm saying you'd define
> \set PROMPT1 '%a%b%c '
> \set PROMPT2 '%w[%a%b%c %w]'
> 
> and you'd end up with matching indentation on multiline queries.
> 
> I'm not sure that we'd need to make something like this work:
>   PROMPT1="%w[$PROMPT1%w]"
> which I think is what you're saying.

PROMPT2="%w[$PROMPT1%w]", and basically yes.

> We already have "%:PROMPT1:" but that expands to the literal value of
> prompt1, not to the value that prompt1 would expand to:

Yeah, that's not so great for this usage.  I guess "expand variables"
could be a separate useful feature (and patch) all by itself...

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  11. Re: Invisible PROMPT2

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2019-11-14T06:37:55Z

    At Wed, 13 Nov 2019 20:57:04 +0100, David Fetter <david@fetter.org> wrote in 
> On Wed, Nov 13, 2019 at 03:58:38PM -0300, Alvaro Herrera wrote:
> > On 2019-Nov-13, David Fetter wrote:
> > 
> > > On Wed, Nov 13, 2019 at 03:06:08PM -0300, Alvaro Herrera wrote:
> > > > On 2019-Nov-13, David Fetter wrote:
> > > > 
> > > > > On Wed, Nov 13, 2019 at 09:47:01AM -0500, Tom Lane wrote:
> > > > 
> > > > > > > How about a circumfix directive (like the existing %[ ... %])
> > > > > > > that replaces everything inside with whitespace, but keeps the width?
> > 
> > > > This seems way too specific to me.  I like the "circumfix" directive
> > > > better, because it allows one to do more things.  I don't have any
> > > > immediate use for it, but it doesn't seem completely far-fetched that
> > > > there are some.
> > 
> > > So something like %w[...%w] where people could put things like PROMPT1
> > > inside?
> > 
> > Hmm, (I'm not sure your proposed syntax works, but let's assume that
> > it does.)  I'm saying you'd define
> > \set PROMPT1 '%a%b%c '
> > \set PROMPT2 '%w[%a%b%c %w]'
> > 
> > and you'd end up with matching indentation on multiline queries.

This seems assuming %x are a kind of stable (until semicolon)
function. But at least %`..` can be volatile.  So, I think the %w
thing in PROMPT2 should be able to refer the actual prompt string
resulted from PROMPT1.

> > I'm not sure that we'd need to make something like this work:
> >   PROMPT1="%w[$PROMPT1%w]"
> > which I think is what you're saying.
> 
> PROMPT2="%w[$PROMPT1%w]", and basically yes.

Like this. Or may be a bit too-much and I don't came up with a
lialistic use-case, but I think of the following syntax.

\set PROMPT1 '%w[%a%b%c%w] '
\set PROMPT2 '%w '

where %w in PROMPT2 is replaced by a whitespace with the same length
to the output of %w[..%w] part in PROMPT1.

> > We already have "%:PROMPT1:" but that expands to the literal value of
> > prompt1, not to the value that prompt1 would expand to:
> 
> Yeah, that's not so great for this usage.  I guess "expand variables"
> could be a separate useful feature (and patch) all by itself...

+1.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  12. Re: Invisible PROMPT2

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-14T14:58:03Z

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> This seems assuming %x are a kind of stable (until semicolon)
> function. But at least %`..` can be volatile.  So, I think the %w
> thing in PROMPT2 should be able to refer the actual prompt string
> resulted from PROMPT1.

Oh, that's a good point.  But it actually leads to a much simpler
definition and implementation than the other ideas we've kicked
around: define %w as "whitespace equal to the length of the
last-generated PROMPT1 string (initially empty)", and we just
have to save PROMPT1 each time we generate it.

Except ... I'm not sure how to deal with hidden escape sequences.
We should probably assume that anything inside %[...%] has width
zero, but how would we remember that?

Maybe count the width of non-escape characters whenever we
generate PROMPT1, and just save that number not the string?
It'd add overhead that's useless when there's no %w, but
probably not enough to care about.

			regards, tom lane

  13. Re: Invisible PROMPT2

    Thomas Munro <thomas.munro@gmail.com> — 2019-11-17T21:11:36Z

    On Fri, Nov 15, 2019 at 3:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Kyotaro Horiguchi <horikyota.ntt@gmail.com> writes:
> > This seems assuming %x are a kind of stable (until semicolon)
> > function. But at least %`..` can be volatile.  So, I think the %w
> > thing in PROMPT2 should be able to refer the actual prompt string
> > resulted from PROMPT1.
>
> Oh, that's a good point.  But it actually leads to a much simpler
> definition and implementation than the other ideas we've kicked
> around: define %w as "whitespace equal to the length of the
> last-generated PROMPT1 string (initially empty)", and we just
> have to save PROMPT1 each time we generate it.
>
> Except ... I'm not sure how to deal with hidden escape sequences.
> We should probably assume that anything inside %[...%] has width
> zero, but how would we remember that?
>
> Maybe count the width of non-escape characters whenever we
> generate PROMPT1, and just save that number not the string?
> It'd add overhead that's useless when there's no %w, but
> probably not enough to care about.

Nice idea.  Here's one like that, that just does the counting at the
end and looks out for readline control codes.  It's pretty naive about
what "width" means though: you'll get two spaces for UTF-8 encoded é,
and I suppose a complete implementation would know about the half
width/full width thing for Chinese and Japanese etc.

  14. Re: Invisible PROMPT2

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-11-18T00:49:24Z

    On 2019-Nov-18, Thomas Munro wrote:

> Nice idea.  Here's one like that, that just does the counting at the
> end and looks out for readline control codes.  It's pretty naive about
> what "width" means though: you'll get two spaces for UTF-8 encoded é,
> and I suppose a complete implementation would know about the half
> width/full width thing for Chinese and Japanese etc.

Hmm ... is this related to what Juan José posted at
https://postgr.es/m/CAC+AXB28ADgwdNRA=aAoWDYPqO1DZR+5NTO8iXGSsFrXyVpqYQ@mail.gmail.com
?  That's backend code of course, though.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  15. Re: Invisible PROMPT2

    Thomas Munro <thomas.munro@gmail.com> — 2019-11-18T01:40:50Z

    On Mon, Nov 18, 2019 at 1:49 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> On 2019-Nov-18, Thomas Munro wrote:
> > Nice idea.  Here's one like that, that just does the counting at the
> > end and looks out for readline control codes.  It's pretty naive about
> > what "width" means though: you'll get two spaces for UTF-8 encoded é,
> > and I suppose a complete implementation would know about the half
> > width/full width thing for Chinese and Japanese etc.
>
> Hmm ... is this related to what Juan José posted at
> https://postgr.es/m/CAC+AXB28ADgwdNRA=aAoWDYPqO1DZR+5NTO8iXGSsFrXyVpqYQ@mail.gmail.com
> ?  That's backend code of course, though.

Yeah.  Maybe pg_wcswidth() would be OK though, and it's available in
psql, though I guess you'd have to make a copy with the escaped bits
stripped out.

  16. Re: Invisible PROMPT2

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-18T17:21:53Z

    Thomas Munro <thomas.munro@gmail.com> writes:
> On Mon, Nov 18, 2019 at 1:49 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>> On 2019-Nov-18, Thomas Munro wrote:
>>> Nice idea.  Here's one like that, that just does the counting at the
>>> end and looks out for readline control codes.  It's pretty naive about
>>> what "width" means though: you'll get two spaces for UTF-8 encoded é,
>>> and I suppose a complete implementation would know about the half
>>> width/full width thing for Chinese and Japanese etc.

> Yeah.  Maybe pg_wcswidth() would be OK though, and it's available in
> psql, though I guess you'd have to make a copy with the escaped bits
> stripped out.

Right, you should use pg_wcswidth() or the underlying PQdsplen() function
to compute display width.  The latter might be more convenient since
you could apply it character by character rather than making a copy
of the string.

			regards, tom lane

  17. Re: Invisible PROMPT2

    Thomas Munro <thomas.munro@gmail.com> — 2019-11-18T21:07:33Z

    On Tue, Nov 19, 2019 at 6:21 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > Yeah.  Maybe pg_wcswidth() would be OK though, and it's available in
> > psql, though I guess you'd have to make a copy with the escaped bits
> > stripped out.
>
> Right, you should use pg_wcswidth() or the underlying PQdsplen() function
> to compute display width.  The latter might be more convenient since
> you could apply it character by character rather than making a copy
> of the string.

Right, a PQdsplen()/PQmblen() loop works nicely, as attached.

I spotted a potential problem: I suppose I could write a PROMPT1 that
includes an invalid multibyte sequence at the end of the buffer and
trick PQmblen() or PQdsplen() into reading a few bytes past the end.
Two defences against that would be (1) use pg_encoding_verifymb()
instead of PQmblen() and (2) use pg_encoding_max_length() to make sure
you can't get close enough to the end of the buffer, but neither of
those functions are available to psql.

  18. Re: Invisible PROMPT2

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-18T23:09:05Z

    Thomas Munro <thomas.munro@gmail.com> writes:
> Right, a PQdsplen()/PQmblen() loop works nicely, as attached.

> I spotted a potential problem: I suppose I could write a PROMPT1 that
> includes an invalid multibyte sequence at the end of the buffer and
> trick PQmblen() or PQdsplen() into reading a few bytes past the end.
> Two defences against that would be (1) use pg_encoding_verifymb()
> instead of PQmblen() and (2) use pg_encoding_max_length() to make sure
> you can't get close enough to the end of the buffer, but neither of
> those functions are available to psql.

You should follow the logic in pg_wcswidth: compute PQmblen() first,
and bail out if it's more than the remaining string length, otherwise
it's ok to apply PQdsplen().

It might be a good idea to explicitly initialize last_prompt1_width to
zero, for clarity.

Should the user docs explicitly say "of the same width as the most recent
output of PROMPT1", as you have in the comments?  That seems a more
precise specification, and it will eliminate some questions people will
otherwise ask.

LGTM otherwise.

			regards, tom lane

  19. Re: Invisible PROMPT2

    Thomas Munro <thomas.munro@gmail.com> — 2019-11-19T03:02:48Z

    On Tue, Nov 19, 2019 at 12:09 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> You should follow the logic in pg_wcswidth: compute PQmblen() first,
> and bail out if it's more than the remaining string length, otherwise
> it's ok to apply PQdsplen().

Got it.  I was worried that it wasn't safe to call even PQmblen(),
because I didn't know a fact about all encodings: as described in the
comment of pg_gb18030_mblen(), all implementations read only the first
byte to determine the length, except for GB18030 which reads the
second byte too, and that's OK because there's always a null
terminator.

> It might be a good idea to explicitly initialize last_prompt1_width to
> zero, for clarity.
>
> Should the user docs explicitly say "of the same width as the most recent
> output of PROMPT1", as you have in the comments?  That seems a more
> precise specification, and it will eliminate some questions people will
> otherwise ask.
>
> LGTM otherwise.

Done, and pushed.  I also skipped negative results from PQdsplen like
pg_wcswidth() does (that oversight explained why a non-readline build
showed the correct alignment for PROMPT1 '%[%033[1m%]%M
%n@%/%R%[%033[0m%]%# ' by strange concindence).

Thanks all for the feedback.  I think the new bikeshed colour looks good.

  20. Re: Invisible PROMPT2

    David Fetter <david@fetter.org> — 2019-11-19T21:37:26Z

    On Tue, Nov 19, 2019 at 04:02:48PM +1300, Thomas Munro wrote:
> On Tue, Nov 19, 2019 at 12:09 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > You should follow the logic in pg_wcswidth: compute PQmblen() first,
> > and bail out if it's more than the remaining string length, otherwise
> > it's ok to apply PQdsplen().
> 
> Got it.  I was worried that it wasn't safe to call even PQmblen(),
> because I didn't know a fact about all encodings: as described in the
> comment of pg_gb18030_mblen(), all implementations read only the first
> byte to determine the length, except for GB18030 which reads the
> second byte too, and that's OK because there's always a null
> terminator.
> 
> > It might be a good idea to explicitly initialize last_prompt1_width to
> > zero, for clarity.
> >
> > Should the user docs explicitly say "of the same width as the most recent
> > output of PROMPT1", as you have in the comments?  That seems a more
> > precise specification, and it will eliminate some questions people will
> > otherwise ask.
> >
> > LGTM otherwise.
> 
> Done, and pushed.  I also skipped negative results from PQdsplen like
> pg_wcswidth() does (that oversight explained why a non-readline build
> showed the correct alignment for PROMPT1 '%[%033[1m%]%M
> %n@%/%R%[%033[0m%]%# ' by strange concindence).
> 
> Thanks all for the feedback.  I think the new bikeshed colour looks good.

Please find attached some polka dots for the bike shed :)

Best,
David.
-- 
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

  21. Re: Invisible PROMPT2

    Maxence Ahlouche <maxence.ahlouche@gmail.com> — 2019-11-27T15:30:12Z

    Hi,

I noticed that this patch does not work when PROMPT1 contains a new line,
since the whole length of PROMPT1 is taken into account for the length of
%w.
Attached screenshot shows the issue on my psql, with the following PROMPT
variables (colors edited out for readability):

\set PROMPT1 '\n[pid:%p] %n :: %`hostname`:%> ‹%/› \n› '
\set PROMPT2 '%w'

Notice in the screenshot that just after inputting a newline, my cursor is
far to the right.
The length of %w should probably be computed starting from the last newline
in PROMPT1.

I could technically get rid of my newline, but since my prompt can get
pretty long, i like the comfort of having my first line of sql start right
at the left of my terminal.

Also attached is a trivial patch to fix this issue, which I have not
extensively tested (works for me at least), and might not be the right way
to do it, but it's a start.
Otherwise, nice feature, I like it!

Regards,
Maxence

  22. Re: Invisible PROMPT2

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-11-27T16:09:44Z

    Maxence Ahlouche <maxence.ahlouche@gmail.com> writes:
> The length of %w should probably be computed starting from the last newline
> in PROMPT1.

Good idea, but I think you need to account for "visible" (ie, if the
newline is inside RL_PROMPT_START_IGNORE, it shouldn't change the width).
It might be best to add logic inside the existing "if (visible)" instead
of making a new top-level case.

Another special case that somebody's likely to whine about is \t, though
to handle that we'd have to make assumptions about the tab stop distance.
Maybe assuming that it's 8 is good enough.

			regards, tom lane

  23. Re: Invisible PROMPT2

    Maxence Ahlouche <maxence.ahlouche@gmail.com> — 2019-12-22T16:43:26Z

    On Wed, 27 Nov 2019 at 17:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Good idea, but I think you need to account for "visible" (ie, if the
> newline is inside RL_PROMPT_START_IGNORE, it shouldn't change the width).
> It might be best to add logic inside the existing "if (visible)" instead
> of making a new top-level case.
>

Right, I assumed that it was safe given that only terminal control
characters were invisible.
Since the title of the terminal window can be changed as well via control
characters, it's probably better not to make that assumption.

I updated the patch accordingly.


> Another special case that somebody's likely to whine about is \t, though
> to handle that we'd have to make assumptions about the tab stop distance.
> Maybe assuming that it's 8 is good enough.
>

The problem with tabs is that any user can set their tabstops to whatever
they want, and a tab doesn't have a fixed width, it just goes up to the
next tab stop.
One way to do it would be to add tabs wherever necessary in prompt2 to make
sure they have the same size as in prompt1 (a list of numbers of spaces,
which we would concatenate with a tab?), but I'm not sure it's worth the
effort.

  24. Re: Invisible PROMPT2

    Thomas Munro <thomas.munro@gmail.com> — 2020-02-10T00:30:39Z

    On Mon, Dec 23, 2019 at 5:43 AM Maxence Ahlouche
<maxence.ahlouche@gmail.com> wrote:
> On Wed, 27 Nov 2019 at 17:09, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Good idea, but I think you need to account for "visible" (ie, if the
>> newline is inside RL_PROMPT_START_IGNORE, it shouldn't change the width).
>> It might be best to add logic inside the existing "if (visible)" instead
>> of making a new top-level case.
>
> Right, I assumed that it was safe given that only terminal control characters were invisible.
> Since the title of the terminal window can be changed as well via control characters, it's probably better not to make that assumption.
>
> I updated the patch accordingly.

Pushed.  Thanks!