Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss
bug#11094: Wrong cursor positioning with display+invisible
0 views
Skip to first unread message
Stefan Monnier
Apr 1, 2012, 9:06:59 PM4/1/12
to Eli Zaretskii, 11...@debbugs.gnu.org
>> Extract the test.cpio and cpio-mode.el files attached, and then try:
>> src/emacs -Q -l .../cpio-mode.el .../test.cpio
> Ouch!
;-)
> (Btw, test.cpio is an invalid archive, according to 2 different
> implementations of the cpio command. The one from GNU/Linux says
> "cpio: premature end of file". Not that it matters for the purposes
> of this discussion.)
I just truncated it, since the original (an initrd) is 10MB long.
>> In Emacs-23, the cursor jumps correctly over the various display and
>> invisible fields
> For some value of "correctly". E.g., position the cursor over the
> ".", which is the first file name in the archive, and type "C-x =".
> You will see "63", which is a lie: the actual value of point is 111.
I don't think it's a lie because that text is preceded by invisible
text, so while "." is at 111, point is indeed at 63 (and that's just
because of adjust_point_for_property, so we can get point to stay at
111 by being more careful with text property's stickiness).
> The old, unidirectional display engine could get away (albeit by the
> skin of its teeth) with such code because it relied on buffer
> positions to increase monotonically with screen positions. So once it
> found a glyph from buffer position N that is greater or equal to the
> value of point, it could place the cursor there, because it "knew" all
> the later glyphs _must_ correspond to positions beyond N. That is why
> it "works" in Emacs 23.
I see that the problem is not so much that all the text is covered by
those properties, but rather that there is are contiguous texts with
`invisible' and `display' properties. I get Emacs-24 to behave
correctly by turning all "invisible span followed by display span" into
just a single display span.
E.g. if you do
emacs -Q
(put-text-property (point-min) (+ 2 (point-min)) 'invisible t)
(put-text-property (+ 2 (point-min)) (+ 4 (point-min)) 'display "<>")
(goto-char (point-min))
you'll see that the cursor is drawn at the wrong place (i.e. after the
<>), whereas if you do
emacs -Q
(put-text-property (point-min) (+ 4 (point-min)) 'display "<>")
(goto-char (point-min))
the cursor is drawn at the right place. I think this is the core of the
problem that's handled differently from Emacs-23.
[ IIUC you've gotten to the same conclusion. ]
> If you still think we should support your original code, we should
> schedule some post-24.1 redesign and refactoring. Let me know what
> you think.
I don't think it's a very high priority problem, but it would be good to
try and tackle it, yes (post-24.1).
Stefan
>> src/emacs -Q -l .../cpio-mode.el .../test.cpio
> Ouch!
;-)
> (Btw, test.cpio is an invalid archive, according to 2 different
> implementations of the cpio command. The one from GNU/Linux says
> "cpio: premature end of file". Not that it matters for the purposes
> of this discussion.)
I just truncated it, since the original (an initrd) is 10MB long.
>> In Emacs-23, the cursor jumps correctly over the various display and
>> invisible fields
> For some value of "correctly". E.g., position the cursor over the
> ".", which is the first file name in the archive, and type "C-x =".
> You will see "63", which is a lie: the actual value of point is 111.
I don't think it's a lie because that text is preceded by invisible
text, so while "." is at 111, point is indeed at 63 (and that's just
because of adjust_point_for_property, so we can get point to stay at
111 by being more careful with text property's stickiness).
> The old, unidirectional display engine could get away (albeit by the
> skin of its teeth) with such code because it relied on buffer
> positions to increase monotonically with screen positions. So once it
> found a glyph from buffer position N that is greater or equal to the
> value of point, it could place the cursor there, because it "knew" all
> the later glyphs _must_ correspond to positions beyond N. That is why
> it "works" in Emacs 23.
I see that the problem is not so much that all the text is covered by
those properties, but rather that there is are contiguous texts with
`invisible' and `display' properties. I get Emacs-24 to behave
correctly by turning all "invisible span followed by display span" into
just a single display span.
E.g. if you do
emacs -Q
(put-text-property (point-min) (+ 2 (point-min)) 'invisible t)
(put-text-property (+ 2 (point-min)) (+ 4 (point-min)) 'display "<>")
(goto-char (point-min))
you'll see that the cursor is drawn at the wrong place (i.e. after the
<>), whereas if you do
emacs -Q
(put-text-property (point-min) (+ 4 (point-min)) 'display "<>")
(goto-char (point-min))
the cursor is drawn at the right place. I think this is the core of the
problem that's handled differently from Emacs-23.
[ IIUC you've gotten to the same conclusion. ]
> If you still think we should support your original code, we should
> schedule some post-24.1 redesign and refactoring. Let me know what
> you think.
I don't think it's a very high priority problem, but it would be good to
try and tackle it, yes (post-24.1).
Stefan
Eli Zaretskii
Apr 1, 2012, 10:56:34 PM4/1/12
to Stefan Monnier, 11...@debbugs.gnu.org
> From: Stefan Monnier <mon...@iro.umontreal.ca>
> Cc: 11...@debbugs.gnu.org
> Date: Sun, 01 Apr 2012 21:06:59 -0400
position is Y, or jump when you move it from one character of a file
name to the next violates the principle of least astonishment, IMO.
> I see that the problem is not so much that all the text is covered by
> those properties, but rather that there is are contiguous texts with
> `invisible' and `display' properties.
Yes, that doesn't help, as I mentioned.
> emacs -Q
> (put-text-property (point-min) (+ 4 (point-min)) 'display "<>")
> (goto-char (point-min))
>
> the cursor is drawn at the right place. I think this is the core of the
> problem that's handled differently from Emacs-23.
> [ IIUC you've gotten to the same conclusion. ]
While there could be more than one way to cut this cake, I still think
we should encourage Lips programmers to use the `cursor' property in
these situations.
> > If you still think we should support your original code, we should
> > schedule some post-24.1 redesign and refactoring. Let me know what
> > you think.
>
> I don't think it's a very high priority problem, but it would be good to
> try and tackle it, yes (post-24.1).
OK.
Should we close this bug, then?
> Cc: 11...@debbugs.gnu.org
> Date: Sun, 01 Apr 2012 21:06:59 -0400
>
> > For some value of "correctly". E.g., position the cursor over the
> > ".", which is the first file name in the archive, and type "C-x =".
> > You will see "63", which is a lie: the actual value of point is 111.
>
> I don't think it's a lie because that text is preceded by invisible
> text, so while "." is at 111, point is indeed at 63 (and that's just
> because of adjust_point_for_property, so we can get point to stay at
> 111 by being more careful with text property's stickiness).
Having point report X when it is position on a character whose
> > For some value of "correctly". E.g., position the cursor over the
> > ".", which is the first file name in the archive, and type "C-x =".
> > You will see "63", which is a lie: the actual value of point is 111.
>
> I don't think it's a lie because that text is preceded by invisible
> text, so while "." is at 111, point is indeed at 63 (and that's just
> because of adjust_point_for_property, so we can get point to stay at
> 111 by being more careful with text property's stickiness).
position is Y, or jump when you move it from one character of a file
name to the next violates the principle of least astonishment, IMO.
> I see that the problem is not so much that all the text is covered by
> those properties, but rather that there is are contiguous texts with
> `invisible' and `display' properties.
> emacs -Q
> (put-text-property (point-min) (+ 4 (point-min)) 'display "<>")
> (goto-char (point-min))
>
> the cursor is drawn at the right place. I think this is the core of the
> problem that's handled differently from Emacs-23.
> [ IIUC you've gotten to the same conclusion. ]
we should encourage Lips programmers to use the `cursor' property in
these situations.
> > If you still think we should support your original code, we should
> > schedule some post-24.1 redesign and refactoring. Let me know what
> > you think.
>
> I don't think it's a very high priority problem, but it would be good to
> try and tackle it, yes (post-24.1).
Should we close this bug, then?
Stefan Monnier
Apr 2, 2012, 9:09:34 AM4/2/12
to Eli Zaretskii, 11...@debbugs.gnu.org
>> > For some value of "correctly". E.g., position the cursor over the
>> > ".", which is the first file name in the archive, and type "C-x =".
>> > You will see "63", which is a lie: the actual value of point is 111.
>> I don't think it's a lie because that text is preceded by invisible
>> text, so while "." is at 111, point is indeed at 63 (and that's just
>> because of adjust_point_for_property, so we can get point to stay at
>> 111 by being more careful with text property's stickiness).
> Having point report X when it is position on a character whose
> position is Y, or jump when you move it from one character of a file
> name to the next violates the principle of least astonishment, IMO.
That's an inevitable consequence of using `invisible'. So yes, I agree
>> > ".", which is the first file name in the archive, and type "C-x =".
>> > You will see "63", which is a lie: the actual value of point is 111.
>> I don't think it's a lie because that text is preceded by invisible
>> text, so while "." is at 111, point is indeed at 63 (and that's just
>> because of adjust_point_for_property, so we can get point to stay at
>> 111 by being more careful with text property's stickiness).
> Having point report X when it is position on a character whose
> position is Y, or jump when you move it from one character of a file
> name to the next violates the principle of least astonishment, IMO.
that `invisible' should be used with great care, but there is no bug in
this particular part (the underlying problem is that the block cursor is
drawn *on* characters whereas point is *between* characters).
>> the cursor is drawn at the right place. I think this is the core of the
>> problem that's handled differently from Emacs-23.
>> [ IIUC you've gotten to the same conclusion. ]
> While there could be more than one way to cut this cake, I still think
> we should encourage Lips programmers to use the `cursor' property in
> these situations.
not sure it would help when point is actually outside of the text area
covered by any of the display strings.
> Should we close this bug, then?
Stefan
Eli Zaretskii
Apr 2, 2012, 12:51:46 PM4/2/12
to Stefan Monnier, 11...@debbugs.gnu.org
> Date: Mon, 02 Apr 2012 09:09:34 -0400
> > Should we close this bug, then?
>
> We might as well keep it open until the problem is actually solved.
Then please define precisely what problem needs to be solved, because
I'm definitely confused about that now.
>
> Hmm... I haven't actually tried to use the `cursor' property, but I'm
> not sure it would help when point is actually outside of the text area
> covered by any of the display strings.
It could do that, if you give the property an integer value.
> Hmm... I haven't actually tried to use the `cursor' property, but I'm
> not sure it would help when point is actually outside of the text area
> covered by any of the display strings.
> > Should we close this bug, then?
>
> We might as well keep it open until the problem is actually solved.
I'm definitely confused about that now.
Stefan Monnier
Apr 2, 2012, 4:15:17 PM4/2/12
to Eli Zaretskii, 11...@debbugs.gnu.org
>> We might as well keep it open until the problem is actually solved.
> Then please define precisely what problem needs to be solved, because
> I'm definitely confused about that now.
We could start with:
> Then please define precisely what problem needs to be solved, because
> I'm definitely confused about that now.
emacs -Q
(put-text-property (point-min) (+ 2 (point-min)) 'invisible t)
(put-text-property (+ 2 (point-min)) (+ 4 (point-min)) 'display "<>")
(goto-char (point-min))
where I'd expect the cursor to be drawn to the left of "<>" rather than
to the right.
Stefan
Eli Zaretskii
Apr 2, 2012, 5:06:51 PM4/2/12
to Stefan Monnier, 11...@debbugs.gnu.org
> From: Stefan Monnier <mon...@IRO.UMontreal.CA>
> Cc: 11...@debbugs.gnu.org
> Date: Mon, 02 Apr 2012 16:15:17 -0400
global-disable-point-adjustment non-nil, I get the cursor where you
want it.
> Cc: 11...@debbugs.gnu.org
> Date: Mon, 02 Apr 2012 16:15:17 -0400
>
> emacs -Q
> (put-text-property (point-min) (+ 2 (point-min)) 'invisible t)
> (put-text-property (+ 2 (point-min)) (+ 4 (point-min)) 'display "<>")
> (goto-char (point-min))
>
> where I'd expect the cursor to be drawn to the left of "<>" rather than
> to the right.
Isn't this an issue with point adjustments? If I set
> emacs -Q
> (put-text-property (point-min) (+ 2 (point-min)) 'invisible t)
> (put-text-property (+ 2 (point-min)) (+ 4 (point-min)) 'display "<>")
> (goto-char (point-min))
>
> where I'd expect the cursor to be drawn to the left of "<>" rather than
> to the right.
global-disable-point-adjustment non-nil, I get the cursor where you
want it.
Stefan Monnier
Apr 3, 2012, 9:37:08 AM4/3/12
to Eli Zaretskii, 11...@debbugs.gnu.org
>> emacs -Q
>> (put-text-property (point-min) (+ 2 (point-min)) 'invisible t)
>> (put-text-property (+ 2 (point-min)) (+ 4 (point-min)) 'display "<>")
>> (goto-char (point-min))
>> where I'd expect the cursor to be drawn to the left of "<>" rather than
>> to the right.
> Isn't this an issue with point adjustments?
No: the (goto-char (point-min)) really moves to (point-min) for me, as
>> (put-text-property (point-min) (+ 2 (point-min)) 'invisible t)
>> (put-text-property (+ 2 (point-min)) (+ 4 (point-min)) 'display "<>")
>> (goto-char (point-min))
>> where I'd expect the cursor to be drawn to the left of "<>" rather than
>> to the right.
> Isn't this an issue with point adjustments?
can be verified with M-: (point).
> If I set global-disable-point-adjustment non-nil, I get the cursor
> where you want it.
after the "<>".
Stefan
Eli Zaretskii
Apr 7, 2012, 8:07:34 AM4/7/12
to Stefan Monnier, 11...@debbugs.gnu.org
> Date: Tue, 03 Apr 2012 09:37:08 -0400
The patch below solves this problem. Do you think it is safe enough
for the release branch?
=== modified file 'src/xdisp.c'
--- src/xdisp.c 2012-03-31 19:30:53 +0000
+++ src/xdisp.c 2012-04-07 11:58:19 +0000
@@ -14042,15 +14042,18 @@ set_cursor_from_row (struct window *w, s
|| pos <= tem)
{
/* If the string from which this glyph came is
- found in the buffer at point, then we've
- found the glyph we've been looking for. If
- it comes from an overlay (tem == 0), and it
- has the `cursor' property on one of its
+ found in the buffer at point, or at position
+ that is closer to point than pos_after, then
+ we've found the glyph we've been looking for.
+ If it comes from an overlay (tem == 0), and
+ it has the `cursor' property on one of its
glyphs, record that glyph as a candidate for
displaying the cursor. (As in the
unidirectional version, we will display the
cursor on the last candidate we find.) */
- if (tem == 0 || tem == pt_old)
+ if (tem == 0
+ || tem == pt_old
+ || (tem - pt_old > 0 && tem < pos_after))
{
/* The glyphs from this string could have
been reordered. Find the one with the
@@ -14088,7 +14091,8 @@ set_cursor_from_row (struct window *w, s
}
}
- if (tem == pt_old)
+ if (tem == pt_old
+ || (tem - pt_old > 0 && tem < pos_after))
goto compute_x;
}
if (tem)
for the release branch?
=== modified file 'src/xdisp.c'
--- src/xdisp.c 2012-03-31 19:30:53 +0000
+++ src/xdisp.c 2012-04-07 11:58:19 +0000
@@ -14042,15 +14042,18 @@ set_cursor_from_row (struct window *w, s
|| pos <= tem)
{
/* If the string from which this glyph came is
- found in the buffer at point, then we've
- found the glyph we've been looking for. If
- it comes from an overlay (tem == 0), and it
- has the `cursor' property on one of its
+ found in the buffer at point, or at position
+ that is closer to point than pos_after, then
+ we've found the glyph we've been looking for.
+ If it comes from an overlay (tem == 0), and
+ it has the `cursor' property on one of its
glyphs, record that glyph as a candidate for
displaying the cursor. (As in the
unidirectional version, we will display the
cursor on the last candidate we find.) */
- if (tem == 0 || tem == pt_old)
+ if (tem == 0
+ || tem == pt_old
+ || (tem - pt_old > 0 && tem < pos_after))
{
/* The glyphs from this string could have
been reordered. Find the one with the
@@ -14088,7 +14091,8 @@ set_cursor_from_row (struct window *w, s
}
}
- if (tem == pt_old)
+ if (tem == pt_old
+ || (tem - pt_old > 0 && tem < pos_after))
goto compute_x;
}
if (tem)
Stefan Monnier
Apr 8, 2012, 10:23:17 PM4/8/12
to Eli Zaretskii, 11...@debbugs.gnu.org
> The patch below solves this problem. Do you think it is safe enough
> for the release branch?
Let's keep it on the trunk for now.
> for the release branch?
Stefan
Eli Zaretskii
Apr 9, 2012, 4:02:24 AM4/9/12
to Stefan Monnier, 11...@debbugs.gnu.org
> Date: Sun, 08 Apr 2012 22:23:17 -0400
Should I close this bug, or is there anything else?
>
> > The patch below solves this problem. Do you think it is safe enough
> > for the release branch?
>
> Let's keep it on the trunk for now.
Done as trunk revision 107809.
> > The patch below solves this problem. Do you think it is safe enough
> > for the release branch?
>
> Let's keep it on the trunk for now.
Should I close this bug, or is there anything else?
Stefan Monnier
Apr 9, 2012, 9:21:19 AM4/9/12
to Eli Zaretskii, 11094...@debbugs.gnu.org
> Should I close this bug, or is there anything else?
Closed, thanks,
Stefan
0 new messages