Hl-line and visual-line

Hl-line and visual-line

[David Reitter]

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

Hl-line highlights the buffer line rather than the visual line even in `visual-line-mode': I don't think that makes sense.

So I think this little change would be good upstream as well.



diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index ebb21c4..4b6d07d 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2010-05-20  David Reitter  <david.reitter@gmail.com>
+
+       * simple.el (visual-line-line-range): Define.
+       (visual-line-mode): Use for hl-line-range-function.
+
 2010-05-07  Chong Yidong  <cyd@stupidchicken.com>
 
        * Version 23.2 released.
diff --git a/lisp/simple.el b/lisp/simple.el
index 28ed4ef..b7f2646 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -4697,6 +4697,11 @@ other purposes."
                            (copy-tree fringe-indicator-alist)))))))
         (set-default symbol value)))
 
+(defun visual-line-line-range ()
+  (save-excursion
+    (cons (progn (vertical-motion 0) (point))
+         (progn (vertical-motion 1) (point)))))
+
 (defvar visual-line--saved-state nil)
 
 (define-minor-mode visual-line-mode
@@ -4712,7 +4717,8 @@ This also turns on `word-wrap' in the buffer."
        ;; visual-line-mode is turned off.
        (dolist (var '(line-move-visual truncate-lines
                       truncate-partial-width-windows
-                      word-wrap fringe-indicator-alist))
+                      word-wrap fringe-indicator-alist
+                      hl-line-range-function))
          (if (local-variable-p var)
              (push (cons var (symbol-value var))
                    visual-line--saved-state)))
@@ -4722,12 +4728,14 @@ This also turns on `word-wrap' in the buffer."
              word-wrap t
              fringe-indicator-alist
              (cons (cons 'continuation visual-line-fringe-indicators)
-                   fringe-indicator-alist)))
+                   fringe-indicator-alist))
+       (set (make-local-variable 'hl-line-range-function) #'visual-line-line-range))
     (kill-local-variable 'line-move-visual)
     (kill-local-variable 'word-wrap)
     (kill-local-variable 'truncate-lines)
     (kill-local-variable 'truncate-partial-width-windows)
     (kill-local-variable 'fringe-indicator-alist)
+    (kill-local-variable 'hl-line-range-function)
     (dolist (saved visual-line--saved-state)
       (set (make-local-variable (car saved)) (cdr saved)))
     (kill-local-variable 'visual-line--saved-state)))


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 203 bytes --]

Re: Hl-line and visual-line

[Eli Zaretskii]

> From: David Reitter <david.reitter@gmail.com>
> Date: Thu, 20 May 2010 16:30:59 -0400
> 
> +(defun visual-line-line-range ()
> +  (save-excursion
> +    (cons (progn (vertical-motion 0) (point))
> +         (progn (vertical-motion 1) (point)))))

This will do The Wrong Thing with bidirectional text, because
vertical-motion puts you on column zero, which is not necessarily the
first character after a newline, in buffer's order of increasing
character positions (a.k.a. "logical order").  The net effect will be
that only part of the screen line will be highlighted.

I just yesterday fixed a similar problem in move-end-of-line (see
revno 100369).  You need to proactively get to the line's first
character, with either skip-chars-backward or (per Stefan's
suggestion) `(forward-line 0)'.

Morale: in Emacs 24, we need to unlearn the seemingly obvious
assumption that the first character at the window margin always
follows the newline of the previous line.  It is no longer true.

Re: Hl-line and visual-line

[David Reitter]

On May 20, 2010, at 5:02 PM, Eli Zaretskii wrote:
>> 
>> +    (cons (progn (vertical-motion 0) (point))
>> +         (progn (vertical-motion 1) (point)))))
> 
> This will do The Wrong Thing with bidirectional text, because
> vertical-motion puts you on column zero, which is not necessarily the
> first character after a newline, in buffer's order of increasing
> character positions (a.k.a. "logical order").  The net effect will be
> that only part of the screen line will be highlighted.
> 
> I just yesterday fixed a similar problem in move-end-of-line (see
> revno 100369).  You need to proactively get to the line's first
> character, with either skip-chars-backward or (per Stefan's
> suggestion) `(forward-line 0)'.

These two seem applicable to buffer lines; I'm not sure how I would do it with word-wrap without `vertical-motion'.

Should I just use `beginning-of-visual-line' and `end-of-visual-line'?

On another note, I first thought of just implementing hl-line in the redisplay code, or perhaps as an option to the cursor display.  When the cursor is drawn, we just highlight the line.  No moving around overlays in Lisp...

Re: Hl-line and visual-line

[Tassilo Horn]

On Friday 21 May 2010 04:35:44 David Reitter wrote:

> On another note, I first thought of just implementing hl-line in the
> redisplay code, or perhaps as an option to the cursor display.  When
> the cursor is drawn, we just highlight the line.  No moving around
> overlays in Lisp...

At a first glance, that would be cool.  I like hl-line-mode, but I have
to disable it in some modes, because the hl-line-overlay's face
overrides the normal font-lock face

Bye,
Tassilo

Re: Hl-line and visual-line

[Eli Zaretskii]

> From: Tassilo Horn <tassilo@member.fsf.org>
> Date: Fri, 21 May 2010 08:34:28 +0200
> 
> On Friday 21 May 2010 04:35:44 David Reitter wrote:
> 
> > On another note, I first thought of just implementing hl-line in the
> > redisplay code, or perhaps as an option to the cursor display.  When
> > the cursor is drawn, we just highlight the line.  No moving around
> > overlays in Lisp...
> 
> At a first glance, that would be cool.  I like hl-line-mode, but I have
> to disable it in some modes, because the hl-line-overlay's face
> overrides the normal font-lock face

Fixing that would mean introducing some feature that will allow you to
request that an overlay's priority is below text properties.  That
sounds like a simpler change (we have negative priority values vacant
for that, I think) than pushing line highlight to the level of the
basic redisplay.  The latter would have significant effect on the
structure of the display code, just like region highlight does.  And
the display engine code is already too complex, to the degree that is
almost unmaintainable.

Re: Hl-line and visual-line

[Tassilo Horn]

On Friday 21 May 2010 10:17:08 Eli Zaretskii wrote:

Hi Eli,

> > > On another note, I first thought of just implementing hl-line in
> > > the redisplay code, or perhaps as an option to the cursor display.
> > > When the cursor is drawn, we just highlight the line.  No moving
> > > around overlays in Lisp...
> > 
> > At a first glance, that would be cool.  I like hl-line-mode, but I
> > have to disable it in some modes, because the hl-line-overlay's face
> > overrides the normal font-lock face
> 
> Fixing that would mean introducing some feature that will allow you to
> request that an overlay's priority is below text properties.

That would be as fine as David's suggestion.

> That sounds like a simpler change (we have negative priority values
> vacant for that, I think) than pushing line highlight to the level of
> the basic redisplay.

Negative overlay priorities with that semantics would be very nice.

Bye,
Tassilo

Re: Hl-line and visual-line

[David Reitter]

On May 21, 2010, at 4:17 AM, Eli Zaretskii wrote:
> 
> Fixing that would mean introducing some feature that will allow you to
> request that an overlay's priority is below text properties.  That
> sounds like a simpler change (we have negative priority values vacant
> for that, I think) than pushing line highlight to the level of the
> basic redisplay.  The latter would have significant effect on the
> structure of the display code, just like region highlight does.  And
> the display engine code is already too complex, to the degree that is
> almost unmaintainable.

Well, for that reason my thinking was along the lines of reducing line highlight functionality to changing the background color only, or something like that, but perhaps some users might prefer a larger or bold font (e.g. when you can't see well).

So yes, let's either fix beginning/end-of-visual-line, or provide a function that does the right thing, and base hl-line on that (in visual-line-mode).

Re: Hl-line and visual-line

[Eli Zaretskii]

> From: David Reitter <david.reitter@gmail.com>
> Date: Fri, 21 May 2010 10:22:22 -0400
> Cc: Tassilo Horn <tassilo@member.fsf.org>,
>  emacs-devel@gnu.org
> 
> So yes, let's either fix beginning/end-of-visual-line, or provide a function that does the right thing, and base hl-line on that (in visual-line-mode).

I will wait to see if there are no objections for fixing
beginning/end-of-visual-line for the bidirectional text case.  If no
one objects, I will add it to my todo.

Re: Hl-line and visual-line

[Eli Zaretskii]

> From: David Reitter <david.reitter@gmail.com>
> Date: Fri, 21 May 2010 10:22:22 -0400
> Cc: Tassilo Horn <tassilo@member.fsf.org>,
>  emacs-devel@gnu.org
> 
> So yes, let's either fix beginning/end-of-visual-line, or provide a function that does the right thing, and base hl-line on that (in visual-line-mode).

No one objected, so I guess this is a GO.

I have one request, though: could you (or someone else) please post
requirements for what these functions should do?  That is, of course,
trivial with unidirectional text, but what about reordered text?

Let's say we have buffer text

  abcde ABCDE FGHIJ xyz

which will be displayed under Visual Line mode as

  abcde JIHGF
  EDCBA xyz

Where should beginning/end-of-visual-line put point in each one of
these two screen lines?

(When you answer this question, please think about the possible uses
of these functions: we need to come up with an answer that is useful
in their various use-cases.)

Thanks.

Re: Hl-line and visual-line

[David Reitter]

On May 23, 2010, at 1:26 PM, Eli Zaretskii wrote:
> I have one request, though: could you (or someone else) please post
> requirements for what these functions should do?  That is, of course,
> trivial with unidirectional text, but what about reordered text?
> 
> Let's say we have buffer text
> 
>  abcde ABCDE FGHIJ xyz
> 
> which will be displayed under Visual Line mode as
> 
>  abcde JIHGF
>  EDCBA xyz
> 
> Where should beginning/end-of-visual-line put point in each one of
> these two screen lines?


The first use case would be to jump to beginning and end of a visual line.  That means, if point is at any of {abcde JIHGF}, beginning would be at "a" and the end at " " (between "E " and "F").
If point is at any of {ABCDE xyz}, then beginning would be at "E" and end would be after "z".

This would cover my use case and C-a, C-e.

C-k (kill-visual-line) would probably need to be rewritten anyway.

A quick grep through the Emacs and Aquamacs source codes does not reveal uses in different contexts.


The second use case would be to actually capture a whole line.  I have functions that kill the whole line (from left to right).   They would use `kill-region' from X to Y.   The same goes for hl-line-mode, where ONE overlay is drawn.
So, these functions would need a new function such as "regions-within-visual-line" or so, returning a list of (from . to) regions, or some other means of identifying the region. 

So far for two requirements.

Are we going to have discontinuous regions?  When I set the Mark at "c" in your example, and point at "G", what is highlighted (transient-mark-mode), and what is killed?

I'm not quite sure what the general interface will be for other functions in such situations, so I can't help further with that...  (NB You're throwing some of the most basic assumptions away.   This could get interesting.)

Re: Hl-line and visual-line

[Eli Zaretskii]

> From: David Reitter <david.reitter@gmail.com>
> Date: Sun, 23 May 2010 15:13:42 -0400
> Cc: tassilo@member.fsf.org,
>  emacs-devel@gnu.org
> 
> The first use case would be to jump to beginning and end of a visual line.  That means, if point is at any of {abcde JIHGF}, beginning would be at "a" and the end at " " (between "E " and "F").
> If point is at any of {ABCDE xyz}, then beginning would be at "E" and end would be after "z".

This already works today, so no change seems to be needed to cover
this.

> C-k (kill-visual-line) would probably need to be rewritten anyway.

Why? it seems to work already, killing the visual line.  Am I missing
something?

> The second use case would be to actually capture a whole line.  I have functions that kill the whole line (from left to right).   They would use `kill-region' from X to Y.   The same goes for hl-line-mode, where ONE overlay is drawn.
> So, these functions would need a new function such as "regions-within-visual-line" or so, returning a list of (from . to) regions, or some other means of identifying the region. 

Sorry, I don't get this part.  Can you give an example that uses
kill-region and another one with one overlay for hl-line-mode, and
explain how the list of the form you mention would help?

> Are we going to have discontinuous regions?  When I set the Mark at "c" in your example, and point at "G", what is highlighted (transient-mark-mode), and what is killed?

Both the region and kill commands work in logical order.  So after
C-w, you will see

  ab JIHG xyz

I don't think we should have discontinuous regions.  Their semantics
is not clear (there's more than one way of interpreting them), and no
bidi-aware application I've seen supports them.

Users expect logical-order regions.  Logical-order regions may surprise
the first time you see it, but are easy to get used to, since that's
the order you read the text you mark.

> (NB You're throwing some of the most basic assumptions away.   This could get interesting.)

Yes, I'm in this "interesting" stuff for several months now.

Re: Hl-line and visual-line

[David Reitter]

On May 23, 2010, at 4:33 PM, Eli Zaretskii wrote:
> 
>> The second use case would be to actually capture a whole line.  I have functions that kill the whole line (from left to right).   They would use `kill-region' from X to Y.   The same goes for hl-line-mode, where ONE overlay is drawn.
>> So, these functions would need a new function such as "regions-within-visual-line" or so, returning a list of (from . to) regions, or some other means of identifying the region. 
> 
> Sorry, I don't get this part.  Can you give an example that uses
> kill-region and another one with one overlay for hl-line-mode, and
> explain how the list of the form you mention would help?

Back to your example, buffer text is "abcde ABCDE FGHIJ xyz", displayed with word-wrap as

0) abcde JIHGF
1) EDCBA xyz

Say, point is in line 0, at "c".   Deleting the visual line should delete line 0, which is two portions of text: "abcde" and "FGHIJ".  
My reference to C-k assumed visual-line-mode semantics, as well, so C-k would delete "de" and "FGHIJ".


> Users expect logical-order regions.  Logical-order regions may surprise
> the first time you see it, but are easy to get used to, since that's
> the order you read the text you mark.

Yes, that sounds good.  I was just thinking about how marking a region by mouse would work; I have seen this with bidirectional text in a NS/Cocoa text edit view, and it seems reasonable.  You're probably in a much better position to tell what bidi users would expect.

D

Re: Hl-line and visual-line

[Eli Zaretskii]

> From: David Reitter <david.reitter@gmail.com>
> Date: Sun, 23 May 2010 19:04:02 -0400
> Cc: emacs-devel@gnu.org
> 
> On May 23, 2010, at 4:33 PM, Eli Zaretskii wrote:
> > 
> >> The second use case would be to actually capture a whole line.  I have functions that kill the whole line (from left to right).   They would use `kill-region' from X to Y.   The same goes for hl-line-mode, where ONE overlay is drawn.
> >> So, these functions would need a new function such as "regions-within-visual-line" or so, returning a list of (from . to) regions, or some other means of identifying the region. 
> > 
> > Sorry, I don't get this part.  Can you give an example that uses
> > kill-region and another one with one overlay for hl-line-mode, and
> > explain how the list of the form you mention would help?
> 
> Back to your example, buffer text is "abcde ABCDE FGHIJ xyz", displayed with word-wrap as
> 
> 0) abcde JIHGF
> 1) EDCBA xyz
> 
> Say, point is in line 0, at "c".   Deleting the visual line should delete line 0, which is two portions of text: "abcde" and "FGHIJ".  
> My reference to C-k assumed visual-line-mode semantics, as well, so C-k would delete "de" and "FGHIJ".

Right.  So the main issue here is not where to move point, but rather
how to generate the list of the regions needed for doing something
with the entire visual line.

IOW, we need to introduce a new subroutine (that would produce such a
list), not change beginning/end-of-visual-line.  Do you agree?

Re: Hl-line and visual-line

[Stefan Monnier]

> IOW, we need to introduce a new subroutine (that would produce such a
> list), not change beginning/end-of-visual-line.  Do you agree?

Yes.


        Stefan "who also would be happy to move the region-highlighting
                code from C to Elisp, tho it's not really related"

Re: Hl-line and visual-line

[Lennart Borgman]

On Mon, May 24, 2010 at 8:46 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> IOW, we need to introduce a new subroutine (that would produce such a
>> list), not change beginning/end-of-visual-line.  Do you agree?
>
> Yes.


Could we please have `point-at-bovl' and `point-at-eovl' (or the
longer versions `visual-line-beginning-position' etc)?

Re: Hl-line and visual-line

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Mon, 24 May 2010 21:06:11 +0200
> Cc: Eli Zaretskii <eliz@gnu.org>, David Reitter <david.reitter@gmail.com>, emacs-devel@gnu.org
> 
> On Mon, May 24, 2010 at 8:46 PM, Stefan Monnier
> <monnier@iro.umontreal.ca> wrote:
> >> IOW, we need to introduce a new subroutine (that would produce such a
> >> list), not change beginning/end-of-visual-line.  Do you agree?
> >
> > Yes.
> 
> 
> Could we please have `point-at-bovl' and `point-at-eovl' (or the
> longer versions `visual-line-beginning-position' etc)?

I think you have them already: you these are beginning-of-visual-line
and end-of-visual-line.  If these don't fit the bill, please tell what
is your definition of `bovl' and `eovl'.

Re: Hl-line and visual-line

[Lennart Borgman]

On Tue, May 25, 2010 at 12:24 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Lennart Borgman <lennart.borgman@gmail.com>
>> Date: Mon, 24 May 2010 21:06:11 +0200
>> Cc: Eli Zaretskii <eliz@gnu.org>, David Reitter <david.reitter@gmail.com>, emacs-devel@gnu.org
>>
>> On Mon, May 24, 2010 at 8:46 PM, Stefan Monnier
>> <monnier@iro.umontreal.ca> wrote:
>> >> IOW, we need to introduce a new subroutine (that would produce such a
>> >> list), not change beginning/end-of-visual-line.  Do you agree?
>> >
>> > Yes.
>>
>>
>> Could we please have `point-at-bovl' and `point-at-eovl' (or the
>> longer versions `visual-line-beginning-position' etc)?
>
> I think you have them already: you these are beginning-of-visual-line
> and end-of-visual-line.  If these don't fit the bill, please tell what
> is your definition of `bovl' and `eovl'.

I mean the same difference as between beginning-of-line and
line-beginning-position (point-at-bol). (I prefer the shorter versions
here. They are easier to read and easy to understand.)

The first one moves point, but the second does not.

Re: Hl-line and visual-line

[David Reitter]

On May 24, 2010, at 6:37 PM, Lennart Borgman wrote:
>> 
>> is your definition of `bovl' and `eovl'.
> 
> I mean the same difference as between beginning-of-line and
> line-beginning-position (point-at-bol). (I prefer the shorter versions
> here. They are easier to read and easy to understand.)
> 
> The first one moves point, but the second does not.

(save-excursion
	(beginning-of-visual-line)
	(point))

The presence of such functions may reinforce the impression that they could be used as "beg" and "end" parameters to any other function that will process the text between "beg" and "end", in order to make it operate on the visual line.

This works for L2R or even R2L text, but as this thread has made clear, not in the bidirectional case. 

So one would want a warning in their DOC strings, if those functions are needed.

Re: Hl-line and visual-line

[Lennart Borgman]

On Tue, May 25, 2010 at 12:47 AM, David Reitter <david.reitter@gmail.com> wrote:
> On May 24, 2010, at 6:37 PM, Lennart Borgman wrote:
>>>
>>> is your definition of `bovl' and `eovl'.
>>
>> I mean the same difference as between beginning-of-line and
>> line-beginning-position (point-at-bol). (I prefer the shorter versions
>> here. They are easier to read and easy to understand.)
>>
>> The first one moves point, but the second does not.
>
> (save-excursion
>        (beginning-of-visual-line)
>        (point))
>
> The presence of such functions may reinforce the impression that they could be used as "beg" and "end" parameters to any other function that will process the text between "beg" and "end", in order to make it operate on the visual line.
>
> This works for L2R or even R2L text, but as this thread has made clear, not in the bidirectional case.
>
> So one would want a warning in their DOC strings, if those functions are needed.

Good point of course. Maybe instead functions like
min-point-visual-line etc would better? Hm, that would be problematic
too because the direction of text might change from min to max on the
line. I think I am beginning to understand what the details here have
been about.

I think then it might be a bad idea to have those functions I suggest, yes.

A comment about that the text shown between beginning-of-visual-line
and end-... is not necessarily the text between those points when
bidirectional text is in the buffer would be good.

I did not realize that "bidi" meant bidirectional (i.e. text in both
directions) simultaneously before. I can realize Eli had some trouble
with this and the display engine. Great work.

Re: Hl-line and visual-line

[Lennart Borgman]

On Fri, May 21, 2010 at 10:17 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>
>> At a first glance, that would be cool.  I like hl-line-mode, but I have
>> to disable it in some modes, because the hl-line-overlay's face
>> overrides the normal font-lock face
>
> Fixing that would mean introducing some feature that will allow you to
> request that an overlay's priority is below text properties.  That
> sounds like a simpler change (we have negative priority values vacant
> for that, I think) than pushing line highlight to the level of the
> basic redisplay.


Could we please make the change to have overlays with negative
priority below text? I have asked for this several times. What is
holding it back?

Re: Hl-line and visual-line

[Eli Zaretskii]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Fri, 21 May 2010 18:54:12 +0200
> Cc: Tassilo Horn <tassilo@member.fsf.org>, emacs-devel@gnu.org
> 
> Could we please make the change to have overlays with negative
> priority below text? I have asked for this several times. What is
> holding it back?

Lack of volunteers to do this job, I guess.

Re: Hl-line and visual-line

[Stefan Monnier]

> Could we please make the change to have overlays with negative
> priority below text? I have asked for this several times. What is
> holding it back?

Lack of patch?


        Stefan

Re: Hl-line and visual-line

[Lennart Borgman]

On Fri, May 21, 2010 at 10:24 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> Could we please make the change to have overlays with negative
>> priority below text? I have asked for this several times. What is
>> holding it back?
>
> Lack of patch?


Didn't I send something for that long ago? I am not sure anymore.

I am not even sure I understand the code. Is it just a simple change
in face_at_buffer_position in xfaces.c that is needed? Faces are
merged there and merging the faces for overlays with negative
priorities first seems to be what is needed.

However to do that I need Qpriority in xfaces.c. It is however defined
in buffer.c. How should that be handled?

Re: Hl-line and visual-line

[Stefan Monnier]

> Didn't I send something for that long ago? I am not sure anymore.

No idea.

> I am not even sure I understand the code. Is it just a simple change
> in face_at_buffer_position in xfaces.c that is needed?

No.  This is a change to overlays and hence affects all properties, not
just `face'.  Maybe it'll also require changes in xfaces.c as well, but the
core change should be somewhat around the code that implements
functionality like get-char-property.


        Stefan

Re: Hl-line and visual-line

[Lennart Borgman]

On Sat, May 22, 2010 at 2:18 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>> Didn't I send something for that long ago? I am not sure anymore.
>
> No idea.
>
>> I am not even sure I understand the code. Is it just a simple change
>> in face_at_buffer_position in xfaces.c that is needed?
>
> No.  This is a change to overlays and hence affects all properties, not
> just `face'.  Maybe it'll also require changes in xfaces.c as well, but the
> core change should be somewhat around the code that implements
> functionality like get-char-property.

Thanks, forgot. I looked up all places calling sort_overlays and
changed the relevant ones (I skipped mouse overlays).

However my current patch does not work as I expect in one regard: When
I use hi-lock with text properties marking and an overlay with
negative priority behind the text the hi-lock faces seems to disappear
when I mark again with hi-lock. I can't figure out what might be
wrong. Any ideas?

Re: Hl-line and visual-line

[Lennart Borgman]

On Sat, May 22, 2010 at 4:35 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
>
> However my current patch does not work as I expect in one regard: When
> I use hi-lock with text properties marking and an overlay with
> negative priority behind the text the hi-lock faces seems to disappear
> when I mark again with hi-lock. I can't figure out what might be
> wrong. Any ideas?

Just my bad testing. Just need to collect the patch.

Re: Hl-line and visual-line

[Lennart Borgman]

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

On Sat, May 22, 2010 at 4:35 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Sat, May 22, 2010 at 2:18 AM, Stefan Monnier
> <monnier@iro.umontreal.ca> wrote:
>>> Didn't I send something for that long ago? I am not sure anymore.
>>
>> No idea.
>>
>>> I am not even sure I understand the code. Is it just a simple change
>>> in face_at_buffer_position in xfaces.c that is needed?
>>
>> No.  This is a change to overlays and hence affects all properties, not
>> just `face'.  Maybe it'll also require changes in xfaces.c as well, but the
>> core change should be somewhat around the code that implements
>> functionality like get-char-property.
>
> Thanks, forgot. I looked up all places calling sort_overlays and
> changed the relevant ones (I skipped mouse overlays).


I have attached the patch. Could someone please check and install it
to the trunk (unless there are no objections).


PS: If someone could tell me how I could do this from my checkout from
Launchpad I would be glad to do it myself the next time.

[-- Attachment #2: overlay-neg-priority-1.diff --]
[-- Type: text/x-patch, Size: 7415 bytes --]





=== modified file 'src/textprop.c'
--- trunk/src/textprop.c	2010-01-19 13:16:01 +0000
+++ patched/src/textprop.c	2010-05-22 01:39:11 +0000
@@ -629,6 +629,11 @@
      Lisp_Object *overlay;
 {
   struct window *w = 0;
+  Lisp_Object tem = Qnil;
+  Lisp_Object tem_text = Qnil;
+  int priority;
+  int noverlays;
+  Lisp_Object *overlay_vec;

   CHECK_NUMBER_COERCE_MARKER (position);

@@ -642,8 +647,6 @@
     }
   if (BUFFERP (object))
     {
-      int noverlays;
-      Lisp_Object *overlay_vec;
       struct buffer *obuf = current_buffer;

       if (XINT (position) < BUF_BEGV (XBUFFER (object))
@@ -658,26 +661,37 @@
       set_buffer_temp (obuf);

       /* Now check the overlays in order of decreasing priority.  */
-      while (--noverlays >= 0)
+      while ( NILP (tem) && (--noverlays >= 0) )
 	{
-	  Lisp_Object tem = Foverlay_get (overlay_vec[noverlays], prop);
+	  tem = Foverlay_get (overlay_vec[noverlays], prop);
 	  if (!NILP (tem))
 	    {
-	      if (overlay)
-		/* Return the overlay we got the property from.  */
-		*overlay = overlay_vec[noverlays];
-	      return tem;
+              Lisp_Object tem_pri = Foverlay_get (overlay_vec[noverlays], Qpriority);
+              priority = (INTEGERP (tem_pri)) ? XINT (tem_pri) : 0;
 	    }
 	}
     }

-  if (overlay)
-    /* Indicate that the return value is not from an overlay.  */
-    *overlay = Qnil;
+  /* Check what to return. */
+  tem_text = Fget_text_property (position, prop, object);
+  if ( (!NILP (tem)) && ( (priority >= 0)
+                          || NILP (tem_text) ))
+    {
+      if (overlay)
+        /* Return the overlay we got the property from.  */
+        *overlay = overlay_vec[noverlays];
+      return tem;
+    }
+  else
+    {
+      if (overlay)
+        /* Indicate that the return value is not from an overlay.  */
+        *overlay = Qnil;

-  /* Not a buffer, or no appropriate overlay, so fall through to the
-     simpler case.  */
-  return Fget_text_property (position, prop, object);
+      /* Not a buffer, or no appropriate overlay, so fall through to the
+         simpler case.  */
+      return tem_text;
+    }
 }

 DEFUN ("get-char-property", Fget_char_property, Sget_char_property, 2, 3, 0,





=== modified file 'src/editfns.c'
--- trunk/src/editfns.c	2010-03-24 18:02:56 +0000
+++ patched/src/editfns.c	2010-05-22 01:48:44 +0000
@@ -444,7 +444,9 @@
     {
       int posn = XINT (position);
       int noverlays;
-      Lisp_Object *overlay_vec, tem;
+      Lisp_Object *overlay_vec;
+      Lisp_Object tem = Qnil;
+      Lisp_Object tem_pri;
       struct buffer *obuf = current_buffer;

       set_buffer_temp (XBUFFER (object));
@@ -466,7 +468,7 @@
       set_buffer_temp (obuf);

       /* Now check the overlays in order of decreasing priority.  */
-      while (--noverlays >= 0)
+      while ( NILP (tem) && (--noverlays >= 0) )
 	{
 	  Lisp_Object ol = overlay_vec[noverlays];
 	  tem = Foverlay_get (ol, prop);
@@ -478,24 +480,28 @@
 		   && XMARKER (start)->insertion_type == 1)
 		  || (OVERLAY_POSITION (finish) == posn
 		      && XMARKER (finish)->insertion_type == 0))
-		; /* The overlay will not cover a char inserted at point.  */
+		tem = Qnil; /* The overlay will not cover a char inserted at point.  */
 	      else
 		{
-		  return tem;
+                  tem_pri = Foverlay_get (ol, Qpriority);
+                  if (tem_pri >= 0) return tem;
 		}
 	    }
 	}

       { /* Now check the text properties.  */
 	int stickiness = text_property_stickiness (prop, position, object);
+        Lisp_Object tem_text = Qnil;
 	if (stickiness > 0)
-	  return Fget_text_property (position, prop, object);
+	  tem_text = Fget_text_property (position, prop, object);
 	else if (stickiness < 0
 		 && XINT (position) > BUF_BEGV (XBUFFER (object)))
-	  return Fget_text_property (make_number (XINT (position) - 1),
+	  tem_text = Fget_text_property (make_number (XINT (position) - 1),
 				     prop, object);
+        if (tem_text && (tem_pri < 0))
+          return tem_text;
 	else
-	  return Qnil;
+	  return tem;
       }
     }
 }





=== modified file 'src/xfaces.c'
--- trunk/src/xfaces.c	2010-01-13 08:35:10 +0000
+++ patched/src/xfaces.c	2010-05-22 02:42:08 +0000
@@ -6278,7 +6278,7 @@
 {
   struct frame *f = XFRAME (w->frame);
   Lisp_Object attrs[LFACE_VECTOR_SIZE];
-  Lisp_Object prop, position;
+  Lisp_Object text_prop, prop, position;
   int i, noverlays;
   Lisp_Object *overlay_vec;
   Lisp_Object frame;
@@ -6286,6 +6286,7 @@
   Lisp_Object propname = mouse ? Qmouse_face : Qface;
   Lisp_Object limit1, end;
   struct face *default_face;
+  int text_merged = 0;

   /* W must display the current buffer.  We could write this function
      to use the frame and buffer of W, but right now it doesn't.  */
@@ -6300,7 +6301,7 @@

   /* Get the `face' or `mouse_face' text property at POS, and
      determine the next position at which the property changes.  */
-  prop = Fget_text_property (position, propname, w->buffer);
+  text_prop = Fget_text_property (position, propname, w->buffer);
   XSETFASTINT (limit1, (limit < endpos ? limit : endpos));
   end = Fnext_single_property_change (position, propname, w->buffer, limit1);
   if (INTEGERP (end))
@@ -6323,7 +6324,7 @@

   /* Optimize common cases where we can use the default face.  */
   if (noverlays == 0
-      && NILP (prop)
+      && NILP (text_prop)
       && !(pos >= region_beg && pos < region_end))
     return default_face->id;

@@ -6331,16 +6332,26 @@
   bcopy (default_face->lface, attrs, sizeof attrs);

   /* Merge in attributes specified via text properties.  */
-  if (!NILP (prop))
-    merge_face_ref (f, prop, attrs, 1, 0);
+  /* if (!NILP (prop)) */
+  /*   merge_face_ref (f, prop, attrs, 1, 0); */

-  /* Now merge the overlay data.  */
+  /* Now merge the overlay data and the text properties.  */
+  if (NILP (text_prop)) text_merged = 1;
   noverlays = sort_overlays (overlay_vec, noverlays, w);
   for (i = 0; i < noverlays; i++)
     {
       Lisp_Object oend;
       int oendpos;

+      if (!text_merged)
+        {
+          int priority = Foverlay_get (overlay_vec[i], Qpriority);
+          if (priority >= 0)
+            {
+              merge_face_ref (f, text_prop, attrs, 1, 0);
+              text_merged = 1;
+            }
+        }
       prop = Foverlay_get (overlay_vec[i], propname);
       if (!NILP (prop))
 	merge_face_ref (f, prop, attrs, 1, 0);
@@ -6350,6 +6361,9 @@
       if (oendpos < endpos)
 	endpos = oendpos;
     }
+  /* If there were no overlays or all had negative prioriteis we have
+     not handled text properties yet. */
+  if (!text_merged) merge_face_ref (f, text_prop, attrs, 1, 0);

   /* If in the region, merge in the region face.  */
   if (pos >= region_beg && pos < region_end)





=== modified file 'src/lisp.h'
--- trunk/src/lisp.h	2010-05-15 21:19:05 +0000
+++ patched/src/lisp.h	2010-05-22 13:50:56 +0000
@@ -2973,1 +2973,1 @@
 extern void set_time_zone_rule P_ ((char *));

 /* defined in buffer.c */
+extern Lisp_Object Qpriority;
 extern int mouse_face_overlay_overlaps P_ ((Lisp_Object));
 extern void nsberror P_ ((Lisp_Object)) NO_RETURN;
 EXFUN (Fset_buffer_multibyte, 1);

Re: Hl-line and visual-line

[Eli Zaretskii]

> From: David Reitter <david.reitter@gmail.com>
> Date: Thu, 20 May 2010 22:35:44 -0400
> Cc: emacs-devel@gnu.org
> 
> > I just yesterday fixed a similar problem in move-end-of-line (see
> > revno 100369).  You need to proactively get to the line's first
> > character, with either skip-chars-backward or (per Stefan's
> > suggestion) `(forward-line 0)'.
> 
> These two seem applicable to buffer lines; I'm not sure how I would do it with word-wrap without `vertical-motion'.

You can't, at least not in general; see below.  But at the very least,
this change shouldn't break highlighting buffer lines, which currently
works well with bidirectional text.

> Should I just use `beginning-of-visual-line' and `end-of-visual-line'?

You could, if we decide that these two should have the semantics of
moving to the first/last character in the logical order.  If we do
decide this, I could try fixing these to DTRT, because currently they
work well only for physical (buffer) lines, not in Visual Line mode
and not in continued lines.

This calls for a more general discussion about the meaning of columns
and related primitives, which I have been postponing for too long.  I
will start a separate thread on that soon.

But the graver problem is that, with bidirectional text, you in
general need more than a single overlay to cover a single visual line.
An example will explain why.  Imagine a buffer whose text is

  abcdefg ABCDEFG XYZ xyz

where upper-case letters represent right-to-left text.  This text will
be reordered for display to produce the following visual line:

  abcdefg ZYX GFEDCBA xyz

In a narrow enough window and under Visual Line mode, it will be
displayed as

  abcdefg ZYX
  GFEDCBA xyz

Since overlays and text properties cover characters in their buffer
order (and that really cannot be changed without terrible user-level
misfeatures), I hope it is clear now that to highlight the second
visual line, you need 2 overlays, covering the characters as shown by
underlining below:

  abcdefg ABCDEFG XYZ xyz
          -------     ---

> On another note, I first thought of just implementing hl-line in the redisplay code, or perhaps as an option to the cursor display.  When the cursor is drawn, we just highlight the line.  No moving around overlays in Lisp...

I think moving features from Lisp to C is currently being frowned
upon.  And in any case, doing this is not as simple as it sounds,
because highlighting must be noted and acted upon when the characters
are examined for their display in each line, which is well before the
cursor is placed.  That's because faces and overlays can in principle
change character dimensions and affect the display in the more global
sense.  So the display engine examines overlays and text properties
when it lays out the characters, and only after a line is laid out, it
considers whether the cursor should be placed on the line, and where.