zsh-workers
 help / color / mirror / code / Atom feed
* [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected
@ 2018-12-06  7:03 Sebastian Gniazdowski
  2018-12-07  1:42 ` Sebastian Gniazdowski
  0 siblings, 1 reply; 9+ messages in thread
From: Sebastian Gniazdowski @ 2018-12-06  7:03 UTC (permalink / raw)
  To: Zsh hackers list

Hello,
if the first character at which "$start $end standout" should be
applied, i.e. $start character, is already highlighted, the style
standout will not be applied to it:

http://psprint.blinkenshell.org/standout-wrong-1.gif

Confirmation: If I manually force F-Sy-H code to clear previous
region_highlight content's before appliying "$start $end standout" to
region_highlight, then everything works – i.e. also $start character
is highlighted:

http://psprint.blinkenshell.org/standout-ok-2.gif

So basically the problem is: no standout-highlighting of $start
character if region_highlight has already an entry covering the $start
character.

I.e. region_highlight+=( "$start $end standout" ) will not work for
$start character.

A more permanent link for the gifs:

https://github.com/zdharma/fast-syntax-highlighting/issues/92#issuecomment-444768370
--
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin
Blog: http://zdharma.org

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

* Re: [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected
  2018-12-06  7:03 [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected Sebastian Gniazdowski
@ 2018-12-07  1:42 ` Sebastian Gniazdowski
  2018-12-07  7:45   ` Daniel Shahaf
  0 siblings, 1 reply; 9+ messages in thread
From: Sebastian Gniazdowski @ 2018-12-07  1:42 UTC (permalink / raw)
  To: Zsh hackers list

I've tested this further. Turns out that when there is no previous
style applied, a magical from-nowhere inverse mode (i.e. standout)
appears on first selected character:

https://asciinema.org/a/KcB0iZNSx4QsYvHFWCT7kFsV2

The point is: I've changed "standout" to "fg=green", and did = not +=
on region higlight (to cancel any highlighting) – I never did any
"standout"/inverse mode, so from where it comes?

That said, in X04 tests this works (more on the tests in separate
thread) – only 'u' is highlighted with standout. So I wonder what can
lay (an option?) behind in-realword additional-inverse and
in-syntetic-test correct no-additional-inverse. Expecially because
this test also is free from additional-inverse:

widget() { BUFFER="true word2 word3"; region_highlight+=( "0 5 fg=196"
); widgb; }; widgb() { region_highlight=( "2 3 standout" ); };
zle -N widget
bindkey '^T' widget
On Thu, 6 Dec 2018 at 08:03, Sebastian Gniazdowski
<sgniazdowski@gmail.com> wrote:
>
> Hello,
> if the first character at which "$start $end standout" should be
> applied, i.e. $start character, is already highlighted, the style
> standout will not be applied to it:
>
> http://psprint.blinkenshell.org/standout-wrong-1.gif
>
> Confirmation: If I manually force F-Sy-H code to clear previous
> region_highlight content's before appliying "$start $end standout" to
> region_highlight, then everything works – i.e. also $start character
> is highlighted:
>
> http://psprint.blinkenshell.org/standout-ok-2.gif
>
> So basically the problem is: no standout-highlighting of $start
> character if region_highlight has already an entry covering the $start
> character.
>
> I.e. region_highlight+=( "$start $end standout" ) will not work for
> $start character.
>
> A more permanent link for the gifs:
>
> https://github.com/zdharma/fast-syntax-highlighting/issues/92#issuecomment-444768370
> --
> Sebastian Gniazdowski
> News: https://twitter.com/ZdharmaI
> IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin
> Blog: http://zdharma.org



-- 
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin
Blog: http://zdharma.org

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

* Re: [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected
  2018-12-07  1:42 ` Sebastian Gniazdowski
@ 2018-12-07  7:45   ` Daniel Shahaf
  2018-12-07 15:04     ` Sebastian Gniazdowski
  0 siblings, 1 reply; 9+ messages in thread
From: Daniel Shahaf @ 2018-12-07  7:45 UTC (permalink / raw)
  To: zsh-workers

Could you post a self-contained reproduction recipe for the bug?  By
that I mean something that we can copy-paste from the email into a fresh
'zsh -f' instance and observe the bug.

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

* Re: [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected
  2018-12-07  7:45   ` Daniel Shahaf
@ 2018-12-07 15:04     ` Sebastian Gniazdowski
  2018-12-09 18:49       ` Peter Stephenson
  0 siblings, 1 reply; 9+ messages in thread
From: Sebastian Gniazdowski @ 2018-12-07 15:04 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh hackers list

On Fri, 7 Dec 2018 at 08:53, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>
> Could you post a self-contained reproduction recipe for the bug?  By
> that I mean something that we can copy-paste from the email into a fresh
> 'zsh -f' instance and observe the bug.

It's hard because it only happens when using F-Sy-H (and also Z-Sy-H
as one user reported – he reported lack of highlighting of selected
rightmost character to occur in both plugins, I'm assuming changing +=
to = in Z-Sy-H's last command in  _zsh_highlight_apply_zle_highlight
will yield the same results – the boundary, rightmost character will
get highlighted).

I did dumping of region_highlight and there are only 2 entries, fg=196
and the standout, like expected. And yet without "fg=196 "rh-entry the
standaout spans on rightmost char. Not spanning on it when the
preceding "fg=196" entry exists.

But more, the F-Sy-H (and probably also Z-Sy-H) doesn't have right to
work at all, because the indices are WRONGLY calculated. The patch
that is pushed to F-Sy-H does:

-min=$CURSOR max=$MARK
+min=$CURSOR max=$(( MARK + 1 ))

because when rightmost character is a MARK and CURSOR goes on the
left, then the index MUST be 1-higher for the highlight to appear on
rightmost char. So all problems are solved by correct indices
computing, and a puzzle why it works with wrong indices, if there is
no other entry in r_highlight, and only under F-Sy-H.

The commit:
https://github.com/zdharma/fast-syntax-highlighting/commit/8cc7f489ae378606091dbd84345c18c8f7292f0f
-- 
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin
Blog: http://zdharma.org

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

* Re: [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected
  2018-12-07 15:04     ` Sebastian Gniazdowski
@ 2018-12-09 18:49       ` Peter Stephenson
  2018-12-09 19:20         ` Sebastian Gniazdowski
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2018-12-09 18:49 UTC (permalink / raw)
  To: Sebastian Gniazdowski, Daniel Shahaf; +Cc: Zsh hackers list

> Could the problem be fixed, .. ASAP?

I won't be working on this, but I can give some poiners.

You'll need to look in Src/Zle/zle_refresh.c.  You'll see the
region_highlights variable that contains the information.  There are 4
special regions at the start --- see definition of
N_SPECIAL_HIGHLIGHTS.

There are two key points in turning this information into the output.
One is following the comment "Calculate attribute based on region". 
That's where the information gets encoded into the array of characters
to output, which remembers the attributes associated with the character
based on the last thing to change.

The other is inside settextattributes(), where the appropriate sequences
are output.  That's called at various points during the output, but only
if we detect something has changed in them: zwcputc() is the key
location, but you'll see there are others.

The big issue here is that the shell is turning structured information
into a sequence to output --- it's not outputing a character with
particular attributes at a particular location, it's outputing a stream
consisting of a mix of characters and instructions to change
attributes.  So, for example, if two changes logically occur at the some
point which one actually happens is, while supposedly well-defined
(can't remember the rules but I think they're documented, not
necessarily in full detail), is bug-prone.

If you're problem has got to do with line wrap, that's even worse ---
as widely noted in earlier threads, you're then fighting various
terminals and their definitions, too.

Good luck.

pws


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

* Re: [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected
  2018-12-09 18:49       ` Peter Stephenson
@ 2018-12-09 19:20         ` Sebastian Gniazdowski
  2018-12-09 19:48           ` Peter Stephenson
  0 siblings, 1 reply; 9+ messages in thread
From: Sebastian Gniazdowski @ 2018-12-09 19:20 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Daniel Shahaf, Zsh hackers list

On Sun, 9 Dec 2018 at 19:49, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
>
> > Could the problem be fixed, .. ASAP?
>
> I won't be working on this, but I can give some poiners.
>
> You'll need to look in Src/Zle/zle_refresh.c.  You'll see the
> region_highlights variable that contains the information.  There are 4
> special regions at the start --- see definition of
> N_SPECIAL_HIGHLIGHTS.
>
> There are two key points in turning this information into the output.
> One is following the comment "Calculate attribute based on region".
> That's where the information gets encoded into the array of characters
> to output, which remembers the attributes associated with the character
> based on the last thing to change.
>
> The other is inside settextattributes(), where the appropriate sequences
> are output.  That's called at various points during the output, but only
> if we detect something has changed in them: zwcputc() is the key
> location, but you'll see there are others.

I think I've already resolved this in the patch in email "[PATCH] Make
256 color codes be based on zle_highlight array, not on termcap". The
function settextattributes() uses function from prompt.c:
set_colour_attribute(zattr atr, int fg_bg, int flags). It is there
where fg_start_code, etc. are read and set. Also, I think thath
zle_refresh.c code probably falls through to prompt.c –
output_colour(int colour, int fg_bg, int use_tc, int truecol, char
*buf), as I've had to change a condition there to not use termcap for
the 248 colors, but zle_highlight instead.

>
> > Could the problem be fixed, .. ASAP?
>

I was so pressured at this that I've done it myself :) it's in the
"[PATCH] Make 256 ..." email that contains the proposed patch.

-- 
Sebastian Gniazdowski
News: https://twitter.com/ZdharmaI
IRC: https://kiwiirc.com/client/chat.freenode.net:+6697/#zplugin
Blog: http://zdharma.org

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

* Re: [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected
  2018-12-09 19:20         ` Sebastian Gniazdowski
@ 2018-12-09 19:48           ` Peter Stephenson
  2018-12-09 20:19             ` Sebastian Gniazdowski
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Stephenson @ 2018-12-09 19:48 UTC (permalink / raw)
  To: Zsh hackers list

On Sun, 2018-12-09 at 20:20 +0100, Sebastian Gniazdowski wrote:
> I think I've already resolved this in the patch in email "[PATCH] Make
> 256 color codes be based on zle_highlight array, not on termcap".

Ah, just those two lines as below?  I hadn't realised that was the same
issue.  I'm happy to commit them on the basis you've got the most
experience with this support, though I suppose I ought to give Oliver
the chance to comment.

Certainly termcap is a bit defective in this sort of support, so
I can believe it.

pws

diff --git a/Src/prompt.c b/Src/prompt.c
index 568bfc2a9..50c51e479 100644
--- a/Src/prompt.c
+++ b/Src/prompt.c
@@ -2067,6 +2067,11 @@ set_colour_attribute(zattr atr, int fg_bg, int
flags)
     } else if (use_truecolor) {
 	ptr += sprintf(ptr, "8;2;%d;%d;%d", colour >> 16,
 		(colour >> 8) & 0xff, colour & 0xff);
+    } else if (use_truecolor) {
+        ptr += sprintf(ptr, "8;2;%d;%d;%d", colour >> 16,
+                (colour >> 8) & 0xff, colour & 0xff);
+    } else if (colour > 7 && colour <= 255) {
+        ptr += sprintf(ptr, "8;5;%d", colour);
     } else
 	*ptr++ = colour + '0';
     strcpy(ptr, fg_bg_sequences[fg_bg].end);


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

* Re: [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected
  2018-12-09 19:48           ` Peter Stephenson
@ 2018-12-09 20:19             ` Sebastian Gniazdowski
  2018-12-09 20:56               ` Peter Stephenson
  0 siblings, 1 reply; 9+ messages in thread
From: Sebastian Gniazdowski @ 2018-12-09 20:19 UTC (permalink / raw)
  To: Peter Stephenson; +Cc: Zsh hackers list

[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]

Maybe an automatic patch extraction got in the way - I meant the patch in
file 256-....diff.txt that's attached in the other email.

Niedz., 9.12.2018, 20:49: Peter Stephenson <p.w.stephenson@ntlworld.com>
napisał(a):

> On Sun, 2018-12-09 at 20:20 +0100, Sebastian Gniazdowski wrote:
> > I think I've already resolved this in the patch in email "[PATCH] Make
> > 256 color codes be based on zle_highlight array, not on termcap".
>
> Ah, just those two lines as below?  I hadn't realised that was the same
> issue.  I'm happy to commit them on the basis you've got the most
> experience with this support, though I suppose I ought to give Oliver
> the chance to comment.
>
> Certainly termcap is a bit defective in this sort of support, so
> I can believe it.
>
> pws
>
> diff --git a/Src/prompt.c b/Src/prompt.c
> index 568bfc2a9..50c51e479 100644
> --- a/Src/prompt.c
> +++ b/Src/prompt.c
> @@ -2067,6 +2067,11 @@ set_colour_attribute(zattr atr, int fg_bg, int
> flags)
>      } else if (use_truecolor) {
>         ptr += sprintf(ptr, "8;2;%d;%d;%d", colour >> 16,
>                 (colour >> 8) & 0xff, colour & 0xff);
> +    } else if (use_truecolor) {
> +        ptr += sprintf(ptr, "8;2;%d;%d;%d", colour >> 16,
> +                (colour >> 8) & 0xff, colour & 0xff);
> +    } else if (colour > 7 && colour <= 255) {
> +        ptr += sprintf(ptr, "8;5;%d", colour);
>      } else
>         *ptr++ = colour + '0';
>      strcpy(ptr, fg_bg_sequences[fg_bg].end);
>
>

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

* Re: [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected
  2018-12-09 20:19             ` Sebastian Gniazdowski
@ 2018-12-09 20:56               ` Peter Stephenson
  0 siblings, 0 replies; 9+ messages in thread
From: Peter Stephenson @ 2018-12-09 20:56 UTC (permalink / raw)
  To: Zsh hackers list

On Sun, 2018-12-09 at 21:19 +0100, Sebastian Gniazdowski wrote:
> Maybe an automatic patch extraction got in the way - I meant the patch
> in
> file 256-....diff.txt that's attached in the other email.

Found it --- using a new version of a mailer.

pws



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

end of thread, other threads:[~2018-12-09 20:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-06  7:03 [BUG] region_highlight+=( "$start $end standout" ) doesn't work as expected Sebastian Gniazdowski
2018-12-07  1:42 ` Sebastian Gniazdowski
2018-12-07  7:45   ` Daniel Shahaf
2018-12-07 15:04     ` Sebastian Gniazdowski
2018-12-09 18:49       ` Peter Stephenson
2018-12-09 19:20         ` Sebastian Gniazdowski
2018-12-09 19:48           ` Peter Stephenson
2018-12-09 20:19             ` Sebastian Gniazdowski
2018-12-09 20:56               ` Peter Stephenson

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).