move-beginning-of-line misbehaves on wrapped-line invisible text - recent cvs checkout

move-beginning-of-line misbehaves on wrapped-line invisible text - recent cvs checkout

[Ken Manheimer]

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

i think i've found a bug in the behavior of move-beginning-of-line
when crossing invisible text at or beyond the right window edge.  i'm
not sure exactly what is meant by the function's description, so don't
know exactly what the right behavior is, but i know it behaves
inconsistently, which i can demonstrate.

move-beginning-of-line is supposed to move to the beginning of the
current _display_ line.  emphasis on "display" is because i'm not
quite clear what it means.

the attached elisp script sets up a buffer with text where you can
demonstrate the problem for your self (or demonstrate that there's
something weird about my setup or me in general, eg hallucinations:). 
the demonstration includes some description, but for those of you that
don't want to run it:

on lines where invisible text starts near (so the elipses hit) or
beyond the right margin, starting at the end of the line and doing a
move-beginning-of-line (often bound to \C-a), the cursor stops in the
invisible text.  if the place where the cursor stopped is at the right
margin, successive move-beginning-of-line doesn't advance.  if the
cursor stopped somewhere to the right, then the next
move-beginning-of-line will actually get to the real beginning of
line.

the inconsistency there is obvious, but is compounded by the fact that
move-beginning-of-line always moves to the real beginning if no
invisible text is present.

i have noticed in passing some problems with move-end-of-line, but
haven't pinpointed them and wouldn't swear that they exist.

ken
ken.manheimer@gmail.com

[-- Attachment #2: overlay-prob.el --]
[-- Type: application/octet-stream, Size: 1450 bytes --]

(defun construct-overlay-problem ()
  (interactive)
  (let* ((buffer (get-buffer-create "overlay-problem"))
         beg
         offset
         ol)
    (pop-to-buffer buffer)
    (erase-buffer)
    (outline-mode)
    (insert "This may demonstrate a bug in move-beginning-of-line in\n")
    (insert "In my build of emacs 20.0.50, it sometimes doesn't move the\n")
    (insert "cursor through invisible text as it should.  The problems\n")
    (insert "occur when invisible text is at or past the right window margin\n")
    (insert "\n")
    (insert "On each of the following '===' lines, hit \C-a and line end\n")
    (insert "On this line, you should see no problem - cursor goes to bol:\n")
    (construct-overlay-line -5)
    (insert "On this and next line, \C-a stays at the invisible text:")
    (insert "\n")
    (construct-overlay-line -2)
    (construct-overlay-line -1)
    (insert "On this line, it'll take two \C-a's to get to the real bol:")
    (insert "\n")
    (construct-overlay-line 0)))
(defun construct-overlay-line (right-offset)
  (let* ((width (window-width))
         (pre (make-string (+ width right-offset) ?=))
         (string (format "%s|invisible" pre))
         (beg (point))
         (ol-start (+ beg width right-offset 1))
         (ol-end (+ ol-start 10))
         ol)
    (insert string)
    (insert "\n\n")
    (setq ol (make-overlay ol-start ol-end))
    (overlay-put ol 'invisible 'outline)
    ))














[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: move-beginning-of-line misbehaves on wrapped-line invisible text - recent cvs checkout

[Richard M. Stallman]

    move-beginning-of-line is supposed to move to the beginning of the
    current _display_ line.  emphasis on "display" is because i'm not
    quite clear what it means.

I believe the idea is that newlines that don't really appear as such
do not count.

I don't see the problems you reported with the medium-size lines,
but I do see the problem with the longest line.  This seems to fix it.
Does this give good results in general?

*** simple.el	05 Jan 2006 10:48:16 -0500	1.783
--- simple.el	19 Jan 2006 12:11:57 -0500	
***************
*** 3734,3740 ****
    (or arg (setq arg 1))
    (if (/= arg 1)
        (line-move (1- arg) t))
!   (beginning-of-line 1)
    (let ((orig (point)))
      (vertical-motion 0)
      (if (/= orig (point))
--- 3735,3747 ----
    (or arg (setq arg 1))
    (if (/= arg 1)
        (line-move (1- arg) t))
!   
!   ;; Move to beginning-of-line, ignoring fields and invisibles.
!   (skip-chars-backward "^\n")
!   (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
!     (goto-char (previous-char-property-change (1- (point))))
!     (skip-chars-backward "^\n"))
! 
    (let ((orig (point)))
      (vertical-motion 0)
      (if (/= orig (point))

Re: move-beginning-of-line misbehaves on wrapped-line invisible text - recent cvs checkout

[Ken Manheimer]

this works for me!

On 1/19/06, Richard M. Stallman <rms@gnu.org> wrote:
>     move-beginning-of-line is supposed to move to the beginning of the
>     current _display_ line.  emphasis on "display" is because i'm not
>     quite clear what it means.
>
> I believe the idea is that newlines that don't really appear as such
> do not count.
>
> I don't see the problems you reported with the medium-size lines,
> but I do see the problem with the longest line.  This seems to fix it.
> Does this give good results in general?

i see none of the problems that had been showing without the patch.  i
haven't exercised emacs very much with the change, but see that it's
already in CVS.  i'll report if i encounter any problems with it. 
thanks!

ken

> *** simple.el   05 Jan 2006 10:48:16 -0500      1.783
> --- simple.el   19 Jan 2006 12:11:57 -0500
> ***************
> *** 3734,3740 ****
>     (or arg (setq arg 1))
>     (if (/= arg 1)
>         (line-move (1- arg) t))
> !   (beginning-of-line 1)
>     (let ((orig (point)))
>       (vertical-motion 0)
>       (if (/= orig (point))
> --- 3735,3747 ----
>     (or arg (setq arg 1))
>     (if (/= arg 1)
>         (line-move (1- arg) t))
> !
> !   ;; Move to beginning-of-line, ignoring fields and invisibles.
> !   (skip-chars-backward "^\n")
> !   (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
> !     (goto-char (previous-char-property-change (1- (point))))
> !     (skip-chars-backward "^\n"))
> !
>     (let ((orig (point)))
>       (vertical-motion 0)
>       (if (/= orig (point))
>

Re: move-beginning-of-line misbehaves on wrapped-line invisible text - recent cvs checkout

[Ken Manheimer]

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

On 1/22/06, Ken Manheimer <ken.manheimer@gmail.com> wrote:
> this works for me!

... but i've discovered what i think is a bug in rms' new
move-beginning-of-line code.

i'm attaching another bit of elisp to construct a demonstration.  it
appears that move-beginning-of-line will _sometimes_ go to the wrong
place - the beginning of the buffer, in this case - if hidden text
it's traversing begins with a newline.

my best guess at a fix for the recent move-beginning-of-lines would be
to remove the first

  (skip-chars-backward "^\n")

- the one before the loop.  that resolves both this current problem
and the previous one i demonstrated.  unfortunately, i can't be
confident about any fix i invent, because i'm hazy about some of the
contingencies move-beginning-of-line is supposed to handle - i don't
know enough about fields, and am still a bit unclear about some
wording in the function's docstring and rms explanation.

(gmail may be preventing sending of application/octet-stream
attachments, which is how it recognizes .el files, so the attachment
may be omitted.  if so, i'll resend with the attachment as a text
file.)

> On 1/19/06, Richard M. Stallman <rms@gnu.org> wrote:
> >     move-beginning-of-line is supposed to move to the beginning of the
> >     current _display_ line.  emphasis on "display" is because i'm not
> >     quite clear what it means.
> >
> > I believe the idea is that newlines that don't really appear as such
> > do not count.
> >
> > I don't see the problems you reported with the medium-size lines,
> > but I do see the problem with the longest line.  This seems to fix it.
> > Does this give good results in general?
>
> i see none of the problems that had been showing without the patch.  i
> haven't exercised emacs very much with the change, but see that it's
> already in CVS.  i'll report if i encounter any problems with it.
> thanks!
>
> ken
>
> > *** simple.el   05 Jan 2006 10:48:16 -0500      1.783
> > --- simple.el   19 Jan 2006 12:11:57 -0500
> > ***************
> > *** 3734,3740 ****
> >     (or arg (setq arg 1))
> >     (if (/= arg 1)
> >         (line-move (1- arg) t))
> > !   (beginning-of-line 1)
> >     (let ((orig (point)))
> >       (vertical-motion 0)
> >       (if (/= orig (point))
> > --- 3735,3747 ----
> >     (or arg (setq arg 1))
> >     (if (/= arg 1)
> >         (line-move (1- arg) t))
> > !
> > !   ;; Move to beginning-of-line, ignoring fields and invisibles.
> > !   (skip-chars-backward "^\n")
> > !   (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
> > !     (goto-char (previous-char-property-change (1- (point))))
> > !     (skip-chars-backward "^\n"))
> > !
> >     (let ((orig (point)))
> >       (vertical-motion 0)
> >       (if (/= orig (point))
> >
>

[-- Attachment #2: overlay-prob2.el --]
[-- Type: application/octet-stream, Size: 1301 bytes --]

(defvar overlay-prob-category nil
  "Variable for demonstrating overlay problem w/categories")
(setplist 'overlay-prob-category nil)
(put 'overlay-prob-category 'invisible 'outline)
(defun construct-overlay-problem ()
  (interactive)
  (let* ((buffer (get-buffer-create "overlay-problem"))
         beg
         offset
         ol)
    (pop-to-buffer buffer)
    (erase-buffer)
    (outline-mode)
    (insert "This may demonstrate a bug in move-beginning-of-line\n")
    (insert "(actually, previous-char-property-change).  In my emacs 20.0.50\n")
    (insert "build (CVS as of 22Jan2006), a property change is skipped.\n")
    (insert "Follow these instructions for a demonstration.\n")
    (insert "\n")
    (insert "At the end of the last line, after the hidden text, do a\n")
    (insert "move-beginning-of-line")
    (insert (substitute-command-keys "(\\[move-beginning-of-line])\n\n"))
    (insert "In my build, the cursor goes to the beginning of the buffer.\n")
    (insert "\n")
    (insert (substitute-command-keys "Do \\[move-beginning-of-line]"))
    (insert " at the end of this line ")
    (setq beg (point))
    (insert "\n some characters *including leading newline* to hide")
    (setq ol (make-overlay beg (point)))
    (overlay-put ol 'category 'overlay-prob-category)))












[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: move-beginning-of-line misbehaves on wrapped-line invisible text - recent cvs checkout

[Ken Manheimer]

i now have a fix i'm more confident about, because (1) it takes care
of another problem i noticed, even with my previous fix, and (2) it
departs less from the checked-in version of the function, leaving less
room for betrayal of unfamiliar concerns.  here's the diff from
simple.el 1.786:

Index: simple.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/simple.el,v
retrieving revision 1.786
diff -u -u -r1.786 simple.el
--- simple.el	23 Jan 2006 04:05:59 -0000	1.786
+++ simple.el	23 Jan 2006 20:46:34 -0000
@@ -3735,11 +3735,14 @@
   (or arg (setq arg 1))
   (if (/= arg 1)
       (line-move (1- arg) t))
-
+
   ;; Move to beginning-of-line, ignoring fields and invisibles.
   (skip-chars-backward "^\n")
   (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
-    (goto-char (previous-char-property-change (1- (point))))
+    (goto-char (previous-char-property-change
+                (if (line-move-invisible-p (point))
+                    (point)
+                  (1- (point)))))
     (skip-chars-backward "^\n"))

   (let ((orig (point)))
@@ -3747,7 +3750,6 @@
     (if (/= orig (point))
 	(goto-char (constrain-to-field (point) orig (/= arg 1) t nil)))))

-
 ;;; Many people have said they rarely use this feature, and often type
 ;;; it by accident.  Maybe it shouldn't even be on a key.
 (put 'set-goal-column 'disabled t)

Re: move-beginning-of-line misbehaves on wrapped-line invisible text - recent cvs checkout

[Lőrentey Károly]

[-- Attachment #1.1.1: Type: text/plain, Size: 1172 bytes --]

Ken Manheimer <ken.manheimer@gmail.com> writes:
> i now have a fix i'm more confident about, because (1) it takes care
> of another problem i noticed, even with my previous fix, and (2) it
> departs less from the checked-in version of the function, leaving less
> room for betrayal of unfamiliar concerns.

I propose the following change instead, which, in addition to
incorporating your fix, restores the useful special handling of field
boundaries.  As noted in another thread, C-a and C-e behaviour is
currently broken in the minibuffer, in *shell* buffers, in entry
fields in customization buffers, and all other places where fields are
used.  This makes `move-{beginning,end}-of-line' inconsistent with the
original `{beginning,end}-of-line' commands (which are still
available) and the underlying `line-{beginning,end}-position'
functions.

Meanwhile, I committed two (hopefully non-controversial) related fixes
in CVS; the first is for an annoying cursor display problem when point
is just before an ellipsis coming from an invisible overlay.  The
second fixed `constrain-to-field' to always return the right result
when new-pos or old-pos is on a field boundary.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: Type: text/x-patch, Size: 4398 bytes --]

Index: simple.el
===================================================================
RCS file: /cvsroot/emacs/emacs/lisp/simple.el,v
retrieving revision 1.786
diff -c -p -r1.786 simple.el
*** simple.el	23 Jan 2006 04:05:59 -0000	1.786
--- simple.el	26 Jan 2006 02:52:52 -0000
*************** and `current-column' to be able to ignor
*** 3691,3705 ****
  
  (defun move-end-of-line (arg)
    "Move point to end of current line as displayed.
! \(If there's an image in the line, this disregards newlines
! which are part of the text that the image rests on.)
  
  With argument ARG not nil or 1, move forward ARG - 1 lines first.
  If point reaches the beginning or end of buffer, it stops there.
! To ignore intangibility, bind `inhibit-point-motion-hooks' to t."
    (interactive "p")
    (or arg (setq arg 1))
!   (let (done)
      (while (not done)
        (let ((newpos
  	     (save-excursion
--- 3691,3712 ----
  
  (defun move-end-of-line (arg)
    "Move point to end of current line as displayed.
! \(This disregards invisible newlines such as those
! which are part of the text that an image rests on.)
  
  With argument ARG not nil or 1, move forward ARG - 1 lines first.
  If point reaches the beginning or end of buffer, it stops there.
! To ignore intangibility, bind `inhibit-point-motion-hooks' to t.
! 
! This function does not move point across a field boundary unless that
! would move point to a different line than the original, unconstrained
! result.  If N is nil or 1, and a rear-sticky field ends at point,
! the point does not move.  To ignore field boundaries bind
! `inhibit-field-text-motion' to t."
    (interactive "p")
    (or arg (setq arg 1))
!   (let ((orig (point))
! 	done)
      (while (not done)
        (let ((newpos
  	     (save-excursion
*************** To ignore intangibility, bind `inhibit-p
*** 3721,3749 ****
  	      ;; and now we're not really at eol,
  	      ;; keep going.
  	      (setq arg 1)
! 	    (setq done t)))))))
  
  (defun move-beginning-of-line (arg)
    "Move point to beginning of current line as displayed.
! \(If there's an image in the line, this disregards newlines
! which are part of the text that the image rests on.)
  
  With argument ARG not nil or 1, move forward ARG - 1 lines first.
  If point reaches the beginning or end of buffer, it stops there.
! To ignore intangibility, bind `inhibit-point-motion-hooks' to t."
    (interactive "p")
    (or arg (setq arg 1))
    (if (/= arg 1)
        (line-move (1- arg) t))
-   
-   ;; Move to beginning-of-line, ignoring fields and invisibles.
-   (skip-chars-backward "^\n")
-   (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
-     (goto-char (previous-char-property-change (1- (point))))
-     (skip-chars-backward "^\n"))
  
    (let ((orig (point)))
      (vertical-motion 0)
      (if (/= orig (point))
  	(goto-char (constrain-to-field (point) orig (/= arg 1) t nil)))))
  
--- 3728,3764 ----
  	      ;; and now we're not really at eol,
  	      ;; keep going.
  	      (setq arg 1)
! 	    (setq done t)))))
!     (if (/= orig (point))
! 	(goto-char (constrain-to-field (point) orig (/= arg 1) t nil)))))
  
  (defun move-beginning-of-line (arg)
    "Move point to beginning of current line as displayed.
! \(This disregards invisible newlines such as those
! which are part of the text that an image rests on.)
  
  With argument ARG not nil or 1, move forward ARG - 1 lines first.
  If point reaches the beginning or end of buffer, it stops there.
! To ignore intangibility, bind `inhibit-point-motion-hooks' to t.
! 
! This function does not move point across a field boundary unless that
! would move point to a different line than the original, unconstrained
! result.  If N is nil or 1, and a front-sticky field starts at point,
! the point does not move.  To ignore field boundaries bind
! `inhibit-field-text-motion' to t."
    (interactive "p")
    (or arg (setq arg 1))
    (if (/= arg 1)
        (line-move (1- arg) t))
  
    (let ((orig (point)))
+     ;; Move to beginning-of-line, ignoring fields and invisibles.
+     (skip-chars-backward "^\n")
+     (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
+       (goto-char (previous-char-property-change (point)))
+       (skip-chars-backward "^\n"))
      (vertical-motion 0)
      (if (/= orig (point))
  	(goto-char (constrain-to-field (point) orig (/= arg 1) t nil)))))


[-- Attachment #1.1.3: Type: text/plain, Size: 15 bytes --]


-- 
Károly

[-- Attachment #1.2: Type: application/pgp-signature, Size: 188 bytes --]

[-- Attachment #2: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: move-beginning-of-line misbehaves on wrapped-line invisible text - recent cvs checkout

[Ken Manheimer]

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

this is working great for me!  (and i think the
cursor-display-before-ellipsis problem you mention was annoying me,
too, i'm glad it's gone!)

+1 on your version of the move-beginning-of-line, here.

ken
ken.manheimer@gmail.com

On 1/25/06, Lőrentey Károly <lorentey@elte.hu> wrote:
> Ken Manheimer <ken.manheimer@gmail.com> writes:
> > i now have a fix i'm more confident about, because (1) it takes care
> > of another problem i noticed, even with my previous fix, and (2) it
> > departs less from the checked-in version of the function, leaving less
> > room for betrayal of unfamiliar concerns.
>
> I propose the following change instead, which, in addition to
> incorporating your fix, restores the useful special handling of field
> boundaries.  As noted in another thread, C-a and C-e behaviour is
> currently broken in the minibuffer, in *shell* buffers, in entry
> fields in customization buffers, and all other places where fields are
> used.  This makes `move-{beginning,end}-of-line' inconsistent with the
> original `{beginning,end}-of-line' commands (which are still
> available) and the underlying `line-{beginning,end}-position'
> functions.
>
> Meanwhile, I committed two (hopefully non-controversial) related fixes
> in CVS; the first is for an annoying cursor display problem when point
> is just before an ellipsis coming from an invisible overlay.  The
> second fixed `constrain-to-field' to always return the right result
> when new-pos or old-pos is on a field boundary.

[-- Attachment #2: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

Re: move-beginning-of-line misbehaves on wrapped-line invisible text - recent cvs checkout

[Richard M. Stallman]

For the current version of simple.el, is this the change that is needed?

*** simple.el	23 Jan 2006 11:48:39 -0500	1.787
--- simple.el	30 Jan 2006 13:23:53 -0500	
***************
*** 3743,3749 ****
      ;; Move to beginning-of-line, ignoring fields and invisibles.
      (skip-chars-backward "^\n")
      (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
!       (goto-char (previous-char-property-change (1- (point))))
        (skip-chars-backward "^\n"))
  
      ;; Take care of fields.
--- 3743,3749 ----
      ;; Move to beginning-of-line, ignoring fields and invisibles.
      (skip-chars-backward "^\n")
      (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
!       (goto-char (previous-char-property-change (point)))
        (skip-chars-backward "^\n"))
  
      ;; Take care of fields.

Re: move-beginning-of-line misbehaves on wrapped-line invisible text - recent cvs checkout

[Ken Manheimer]

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

this works for me.  it's different from the one that Lőrentey Károly
sent, in response to my patch, which he asserts also provides properly
for field boundaries, and which also works for me.

ken
ken.manheimer@gmail.com

On 1/30/06, Richard M. Stallman <rms@gnu.org> wrote:
> For the current version of simple.el, is this the change that is needed?
>
> *** simple.el   23 Jan 2006 11:48:39 -0500      1.787
> --- simple.el   30 Jan 2006 13:23:53 -0500
> ***************
> *** 3743,3749 ****
>       ;; Move to beginning-of-line, ignoring fields and invisibles.
>       (skip-chars-backward "^\n")
>       (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
> !       (goto-char (previous-char-property-change (1- (point))))
>         (skip-chars-backward "^\n"))
>
>       ;; Take care of fields.
> --- 3743,3749 ----
>       ;; Move to beginning-of-line, ignoring fields and invisibles.
>       (skip-chars-backward "^\n")
>       (while (and (not (bobp)) (line-move-invisible-p (1- (point))))
> !       (goto-char (previous-char-property-change (point)))
>         (skip-chars-backward "^\n"))
>
>       ;; Take care of fields.
>

[-- Attachment #2: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel