zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH v3 try2] prompt: support generic non-visible regions
@ 2023-02-28 15:55 Felipe Contreras
  2023-02-28 16:29 ` Oliver Kiddle
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Felipe Contreras @ 2023-02-28 15:55 UTC (permalink / raw)
  To: zsh-workers; +Cc: Bart Schaefer, Roman Perepelitsa, Felipe Contreras

readline uses \001 (start of header) and \002 (start of text) as markers
to delimit a non-visible character zone, which are necessary to
calculate the width of a prompt.

In zsh we do this with %{ and %}, but we could support
\001 and \002 as well, so that a function which generates output for
the prompt can use colors in a way that works for both bash and zsh.

This additionally has the benefit of allowing prompts without
PROMPT_PERCENT to use colors correctly.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

No change since the previous try.

This is how I tested the nesting:

  $' %{\e[31m\x01\e[31m\x01\e[31m\x02\e[31m\x02\e[31m%}master%{\e[m\x01\e[m\x01\e[m\x02\e[m\x02\e[m%}'

And for the \001 and \002 reference:

https://git.savannah.gnu.org/cgit/readline.git/tree/display.c#n340

 Src/prompt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Src/prompt.c b/Src/prompt.c
index 39fcf5eb7..8d7a38089 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -877,6 +877,16 @@ putpromptchar(int doprint, int endchar)
 		    bv->bp += strlen(bv->bp);
 		}
 	    }
+	} else if(*bv->fm == 0x01) { // start non-visible characters
+	    if (!bv->dontcount++) {
+		addbufspc(1);
+		*bv->bp++ = Inpar;
+	    }
+	} else if(*bv->fm == 0x02) { // end non-visible characters
+	    if (bv->dontcount && !--bv->dontcount) {
+		addbufspc(1);
+		*bv->bp++ = Outpar;
+	    }
 	} else {
 	    char c = *bv->fm == Meta ? *++bv->fm ^ 32 : *bv->fm;
 
-- 
2.39.2



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 try2] prompt: support generic non-visible regions
  2023-02-28 15:55 [PATCH v3 try2] prompt: support generic non-visible regions Felipe Contreras
@ 2023-02-28 16:29 ` Oliver Kiddle
  2023-03-01 12:54   ` Felipe Contreras
  2023-02-28 19:36 ` Bart Schaefer
  2023-03-08  4:04 ` [PATCH v4] " Felipe Contreras
  2 siblings, 1 reply; 5+ messages in thread
From: Oliver Kiddle @ 2023-02-28 16:29 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: zsh-workers, Bart Schaefer, Roman Perepelitsa

Felipe Contreras wrote:
> readline uses \001 (start of header) and \002 (start of text) as markers

So the literal ASCII Ctrl-A and Ctrl-B characters and not the \001 etc
strings, right?

The main concern I have with this is whether there definitely is no use
of those two characters anywhere by any terminal for some other purpose
such that some user somewhere has them already in their prompt? And do
we know they won't form part of some other future terminal sequence? We
can find a special way to generate the literal characters but that only
helps with future uses, not backward compatibility.

I can understand the motivation to be able to support the lack of
prompt_percent and perhaps both bash and zsh in some plugin or other.
That aside, it is somewhat frustrating that zsh doesn't have full
control of terminal attributes in the prompt and I would advise anyone
to use prompt_percent. And in a plugin to add bash and zsh special case
code, even if this is added. Do we really want to encourage the use of
literal escapes?

> +	} else if(*bv->fm == 0x01) { // start non-visible characters

For now, we've not, to my knowledge, bumped our requirements to cover
C99, C17 etc so stick with old-style C comments in the code.*

Oliver

*Except I notice exceptions have crept into Src/Modules/curses.c
If we want to consider this, it should be a separate discussion.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 try2] prompt: support generic non-visible regions
  2023-02-28 15:55 [PATCH v3 try2] prompt: support generic non-visible regions Felipe Contreras
  2023-02-28 16:29 ` Oliver Kiddle
@ 2023-02-28 19:36 ` Bart Schaefer
  2023-03-08  4:04 ` [PATCH v4] " Felipe Contreras
  2 siblings, 0 replies; 5+ messages in thread
From: Bart Schaefer @ 2023-02-28 19:36 UTC (permalink / raw)
  To: zsh-workers

Previous discussion of this starts at workers/50469


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 try2] prompt: support generic non-visible regions
  2023-02-28 16:29 ` Oliver Kiddle
@ 2023-03-01 12:54   ` Felipe Contreras
  0 siblings, 0 replies; 5+ messages in thread
From: Felipe Contreras @ 2023-03-01 12:54 UTC (permalink / raw)
  To: Oliver Kiddle; +Cc: zsh-workers, Bart Schaefer, Roman Perepelitsa

On Tue, Feb 28, 2023 at 10:29 AM Oliver Kiddle <opk@zsh.org> wrote:
>
> Felipe Contreras wrote:
> > readline uses \001 (start of header) and \002 (start of text) as markers
>
> So the literal ASCII Ctrl-A and Ctrl-B characters and not the \001 etc
> strings, right?

Yes.

> The main concern I have with this is whether there definitely is no use
> of those two characters anywhere by any terminal for some other purpose
> such that some user somewhere has them already in their prompt?

If that was an issue, wouldn't bash users have already experienced it
in the years this code has been active? (or possibly decades)

Moreover, can we agree that it's at least extremely unlikely for this
to be an actual issue?

> And do we know they won't form part of some other future terminal
> sequence?

Once again, can we agree that's unlikely? (especially since bash
already uses these)

> We can find a special way to generate the literal characters but that
> only helps with future uses, not backward compatibility.

Like what? A setopt flag?

> I can understand the motivation to be able to support the lack of
> prompt_percent and perhaps both bash and zsh in some plugin or other.

I personally don't care about the prompt_percent stuff, somebody else
mentioned it. The usefulness of sharing prompts between bash and zsh
is what I'm interested in.

> > +     } else if(*bv->fm == 0x01) { // start non-visible characters
>
> For now, we've not, to my knowledge, bumped our requirements to cover
> C99, C17 etc so stick with old-style C comments in the code.*

OK. Will fix in the next version.

-- 
Felipe Contreras


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4] prompt: support generic non-visible regions
  2023-02-28 15:55 [PATCH v3 try2] prompt: support generic non-visible regions Felipe Contreras
  2023-02-28 16:29 ` Oliver Kiddle
  2023-02-28 19:36 ` Bart Schaefer
@ 2023-03-08  4:04 ` Felipe Contreras
  2 siblings, 0 replies; 5+ messages in thread
From: Felipe Contreras @ 2023-03-08  4:04 UTC (permalink / raw)
  To: zsh-workers
  Cc: Bart Schaefer, Roman Perepelitsa, Oliver Kiddle, Felipe Contreras

readline uses \001 (start of header) and \002 (start of text) as markers
to delimit a non-visible character zone, which are necessary to
calculate the width of a prompt.

In zsh we do this with %{ and %}, but we could support
\001 and \002 as well, so that a function which generates output for
the prompt can use colors in a way that works for both bash and zsh.

This additionally has the benefit of allowing prompts without
PROMPT_PERCENT to use colors correctly.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
Interdiff against v3:
  diff --git a/Src/prompt.c b/Src/prompt.c
  index 8d7a38089..2ed90ebe1 100644
  --- a/Src/prompt.c
  +++ b/Src/prompt.c
  @@ -877,12 +877,12 @@ putpromptchar(int doprint, int endchar)
   		    bv->bp += strlen(bv->bp);
   		}
   	    }
  -	} else if(*bv->fm == 0x01) { // start non-visible characters
  +	} else if(*bv->fm == 0x01) { /* start non-visible characters */
   	    if (!bv->dontcount++) {
   		addbufspc(1);
   		*bv->bp++ = Inpar;
   	    }
  -	} else if(*bv->fm == 0x02) { // end non-visible characters
  +	} else if(*bv->fm == 0x02) { /* end non-visible characters */
   	    if (bv->dontcount && !--bv->dontcount) {
   		addbufspc(1);
   		*bv->bp++ = Outpar;

 Src/prompt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Src/prompt.c b/Src/prompt.c
index 39fcf5eb7..2ed90ebe1 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -877,6 +877,16 @@ putpromptchar(int doprint, int endchar)
 		    bv->bp += strlen(bv->bp);
 		}
 	    }
+	} else if(*bv->fm == 0x01) { /* start non-visible characters */
+	    if (!bv->dontcount++) {
+		addbufspc(1);
+		*bv->bp++ = Inpar;
+	    }
+	} else if(*bv->fm == 0x02) { /* end non-visible characters */
+	    if (bv->dontcount && !--bv->dontcount) {
+		addbufspc(1);
+		*bv->bp++ = Outpar;
+	    }
 	} else {
 	    char c = *bv->fm == Meta ? *++bv->fm ^ 32 : *bv->fm;
 
-- 
2.39.2



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-03-08  4:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-28 15:55 [PATCH v3 try2] prompt: support generic non-visible regions Felipe Contreras
2023-02-28 16:29 ` Oliver Kiddle
2023-03-01 12:54   ` Felipe Contreras
2023-02-28 19:36 ` Bart Schaefer
2023-03-08  4:04 ` [PATCH v4] " Felipe Contreras

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).