[BUG] Marker points into wrong buffer

[BUG] Marker points into wrong buffer

[Rasmus]

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

Hi all,

This is a follow-up to Eric F's previous bug-report about getting
"Marker points into wrong buffer" when having Babel #+CALL statements
in Emacs Org-mode.  Here's (one of) the previous messages about the
issue:

     http://permalink.gmane.org/gmane.emacs.orgmode/77515

I bisected emacs-bzr to find the revision where it breaks for me
(cf. attached test case).  It's this one:

    committer: Dmitry Antipov <dmantipov@yandex.ru>
    branch nick: trunk
    timestamp: Thu 2013-08-29 20:36:54 +0400
    message:
    * intervals.c (set_point_from_marker): New function.
    * editfns.c (Fgoto_char):
    * process.c (Finternal_default_process_filter):
    * window.c (select_window_1): Use it.
    * buffer.h (set_point_from_marker): Add prototype.

The test file is test.sh which returns 0 if everything is good.  After
running test one switches revision, which is the job of bzr bisect,
though I was unable to get it working.

If anyone can figure out what is wrong on the Org side or what "broke"
in Emacs-Core it would be great!  "Luckily" it's an all-C commit so I
don't know how to proceed from here. . .

Thanks in advance,
Rasmus

-- 
You people at the NSA are becoming my new best friends!

[-- Attachment #2: r114069.patch --]
[-- Type: application/octet-stream, Size: 4547 bytes --]

------------------------------------------------------------
revno: 114069
committer: Dmitry Antipov <dmantipov@yandex.ru>
branch nick: trunk
timestamp: Thu 2013-08-29 20:36:54 +0400
message:
  * intervals.c (set_point_from_marker): New function.
  * editfns.c (Fgoto_char):
  * process.c (Finternal_default_process_filter):
  * window.c (select_window_1): Use it.
  * buffer.h (set_point_from_marker): Add prototype.
diff:
=== modified file 'src/ChangeLog'
--- src/ChangeLog	2013-08-29 15:32:04 +0000
+++ src/ChangeLog	2013-08-29 16:36:54 +0000
@@ -1,3 +1,11 @@
+2013-08-29  Dmitry Antipov  <dmantipov@yandex.ru>
+
+	* intervals.c (set_point_from_marker): New function.
+	* editfns.c (Fgoto_char):
+	* process.c (Finternal_default_process_filter):
+	* window.c (select_window_1): Use it.
+	* buffer.h (set_point_from_marker): Add prototype.
+
 2013-08-29  Eli Zaretskii  <eliz@gnu.org>
 
 	* w32.c (term_winsock): Call release_listen_threads before calling

=== modified file 'src/buffer.h'
--- src/buffer.h	2013-08-27 18:47:55 +0000
+++ src/buffer.h	2013-08-29 16:36:54 +0000
@@ -249,6 +249,7 @@
 extern void set_point_both (ptrdiff_t, ptrdiff_t);
 extern void temp_set_point_both (struct buffer *,
 				 ptrdiff_t, ptrdiff_t);
+extern void set_point_from_marker (Lisp_Object);
 extern void enlarge_buffer_text (struct buffer *, ptrdiff_t);
 
 \f

=== modified file 'src/editfns.c'
--- src/editfns.c	2013-08-27 18:47:55 +0000
+++ src/editfns.c	2013-08-29 16:36:54 +0000
@@ -233,26 +233,12 @@
 The return value is POSITION.  */)
   (register Lisp_Object position)
 {
-  ptrdiff_t pos;
-
-  if (MARKERP (position)
-      && current_buffer == XMARKER (position)->buffer)
-    {
-      pos = marker_position (position);
-      if (pos < BEGV)
-	SET_PT_BOTH (BEGV, BEGV_BYTE);
-      else if (pos > ZV)
-	SET_PT_BOTH (ZV, ZV_BYTE);
-      else
-	SET_PT_BOTH (pos, marker_byte_position (position));
-
-      return position;
-    }
-
-  CHECK_NUMBER_COERCE_MARKER (position);
-
-  pos = clip_to_bounds (BEGV, XINT (position), ZV);
-  SET_PT (pos);
+  if (MARKERP (position))
+    set_point_from_marker (position);
+  else if (INTEGERP (position))
+    SET_PT (clip_to_bounds (BEGV, XINT (position), ZV));
+  else
+    wrong_type_argument (Qinteger_or_marker_p, position);
   return position;
 }
 

=== modified file 'src/intervals.c'
--- src/intervals.c	2013-06-30 15:14:45 +0000
+++ src/intervals.c	2013-08-29 16:36:54 +0000
@@ -1821,6 +1821,18 @@
   set_point_both (charpos, buf_charpos_to_bytepos (current_buffer, charpos));
 }
 
+/* Set PT from MARKER's clipped position.  */
+
+void
+set_point_from_marker (Lisp_Object marker)
+{
+  if (XMARKER (marker)->buffer != current_buffer)
+    error ("Marker points into wrong buffer");
+  set_point_both
+    (clip_to_bounds (BEGV, marker_position (marker), ZV),
+     clip_to_bounds (BEGV_BYTE, marker_byte_position (marker), ZV_BYTE));
+}
+
 /* If there's an invisible character at position POS + TEST_OFFS in the
    current buffer, and the invisible property has a `stickiness' such that
    inserting a character at position POS would inherit the property it,

=== modified file 'src/process.c'
--- src/process.c	2013-08-27 19:36:28 +0000
+++ src/process.c	2013-08-29 16:36:54 +0000
@@ -5178,15 +5178,10 @@
 
       bset_read_only (current_buffer, Qnil);
 
-      /* Insert new output into buffer
-	 at the current end-of-output marker,
-	 thus preserving logical ordering of input and output.  */
+      /* Insert new output into buffer at the current end-of-output
+	 marker, thus preserving logical ordering of input and output.  */
       if (XMARKER (p->mark)->buffer)
-	SET_PT_BOTH (clip_to_bounds (BEGV,
-				     marker_position (p->mark), ZV),
-		     clip_to_bounds (BEGV_BYTE,
-				     marker_byte_position (p->mark),
-				     ZV_BYTE));
+	set_point_from_marker (p->mark);
       else
 	SET_PT_BOTH (ZV, ZV_BYTE);
       before = PT;

=== modified file 'src/window.c'
--- src/window.c	2013-08-27 03:52:21 +0000
+++ src/window.c	2013-08-29 16:36:54 +0000
@@ -549,15 +549,7 @@
      than one window.  It also matters when
      redisplay_window has altered point after scrolling,
      because it makes the change only in the window.  */
-  {
-    register ptrdiff_t new_point = marker_position (XWINDOW (window)->pointm);
-    if (new_point < BEGV)
-      SET_PT (BEGV);
-    else if (new_point > ZV)
-      SET_PT (ZV);
-    else
-      SET_PT (new_point);
-  }
+  set_point_from_marker (XWINDOW (window)->pointm);
 }
 
 DEFUN ("select-window", Fselect_window, Sselect_window, 1, 2, 0,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: test.sh --]
[-- Type: text/x-sh, Size: 287 bytes --]

#!/bin/bash
make maintainer-clean > /dev/null 2>&1
./autogen.sh  > /dev/null 2>&1
./configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib \
    --localstatedir=/var --without-x --without-sound   > /dev/null 2>&1
make  > /dev/null 2>&1
./src/emacs -l ./test.el --batch | grep 0

[-- Attachment #4: test.org --]
[-- Type: application/vnd.lotus-organizer, Size: 104 bytes --]

[-- Attachment #5: test.el --]
[-- Type: application/emacs-lisp, Size: 229 bytes --]

Re: [BUG][PATCH] Marker points into wrong buffer

[Rasmus]

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

Rasmus <rasmus@gmx.us> writes:

> If anyone can figure out what is wrong on the Org side or what "broke"
> in Emacs-Core it would be great!  "Luckily" it's an all-C commit so I
> don't know how to proceed from here. . .

This really stupid patch allows me to export the document I was unable
to export yesterday with emacs-bzr r115062 (latest or almost latest)
and the latest version Org.

Eric F., would you mind testing it with your slides?

–Rasmus


-- 
C is for Cookie

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Workaround-for-new-marker-behavior.patch --]
[-- Type: text/x-diff, Size: 1028 bytes --]

From 7dcd18bd9df54edbd9395e09459489c35be02c22 Mon Sep 17 00:00:00 2001
From: Rasmus <rasmus@gmx.us>
Date: Tue, 12 Nov 2013 02:27:51 +0100
Subject: [PATCH] Workaround for new marker behavior

* ob-ref.el (org-babel-ref-parse): Fix for Emacs (core) rev. 114069.
---
 lisp/ob-ref.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-ref.el b/lisp/ob-ref.el
index 5a3c8ba..0ccf753 100644
--- a/lisp/ob-ref.el
+++ b/lisp/ob-ref.el
@@ -85,7 +85,10 @@ the variable."
       (cons (intern var)
 	    (let ((out (save-excursion
 			 (when org-babel-current-src-block-location
-			   (goto-char org-babel-current-src-block-location))
+			   (if (version< "24.3.50.1" emacs-version)
+			       (goto-char org-babel-current-src-block-location)
+			     (switch-to-buffer (marker-buffer org-babel-current-src-block-location))
+			     (goto-char (marker-position org-babel-current-src-block-location))))
 			 (org-babel-read ref))))
 	      (if (equal out ref)
 		  (if (string-match "^\".*\"$" ref)
--
1.8.4.2

Re: [BUG][PATCH] Marker points into wrong buffer

[Eric S Fraga]

Rasmus <rasmus@gmx.us> writes:

> Rasmus <rasmus@gmx.us> writes:
>
>> If anyone can figure out what is wrong on the Org side or what "broke"
>> in Emacs-Core it would be great!  "Luckily" it's an all-C commit so I
>> don't know how to proceed from here. . .
>
> This really stupid patch allows me to export the document I was unable
> to export yesterday with emacs-bzr r115062 (latest or almost latest)
> and the latest version Org.
>
> Eric F., would you mind testing it with your slides?

I will be very happy to!  I'm just about to head off to give a 2 hour
lecture and then busy all afternoon with meetings but I'll try to check
this out today at some point.

Thanks,
eric
-- 
: Eric S Fraga (0xFFFCF67D), Emacs 24.3.50.1, Org release_8.2.2-180-g71152d

Re: [BUG][PATCH] Marker points into wrong buffer

[Eric S Fraga]

Rasmus <rasmus@gmx.us> writes:

[...]

> This really stupid patch allows me to export the document I was unable
> to export yesterday with emacs-bzr r115062 (latest or almost latest)
> and the latest version Org.
>
> Eric F., would you mind testing it with your slides?

I've tested it with three files, one a beamer presentation, another a
large LaTeX document and the last one a very small LaTeX file.  The
first with octave code and the other two with emacs lisp.  Two with
call_xxx() construct and one with #+call:.  All cases worked perfectly
fine!

Thanks!

eric
-- 
: Eric S Fraga (0xFFFCF67D), Emacs 24.3.50.1, Org release_8.2.2-181-gf31eb4.dirty

Re: [BUG][PATCH] Marker points into wrong buffer

[Achim Gratz]

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

Rasmus writes:
> Rasmus <rasmus@gmx.us> writes:
>
>> If anyone can figure out what is wrong on the Org side or what "broke"
>> in Emacs-Core it would be great!  "Luckily" it's an all-C commit so I
>> don't know how to proceed from here. . .
>
> This really stupid patch allows me to export the document I was unable
> to export yesterday with emacs-bzr r115062 (latest or almost latest)
> and the latest version Org.

Here's a better patch (I hope).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-ob-ref-Fix-Marker-points-into-wrong-buffer-error.patch --]
[-- Type: text/x-patch, Size: 1228 bytes --]

From c297d59c7ec6ce04ebba3cbeb9641217d1ff5cf1 Mon Sep 17 00:00:00 2001
From: Achim Gratz <Stromeko@Stromeko.DE>
Date: Tue, 12 Nov 2013 21:55:53 +0100
Subject: [PATCH] ob-ref: Fix "Marker points into wrong buffer" error

* lisp/ob-ref.el (org-babel-ref-parse): If
  `org-babel-current-src-block-location' is a marker, it can be from
  another buffer, use marker-position instead in this case.

Introduced with r114064 on Emacs trunk.  Not sure if this is a bug in
Org or Emacs, but the patch restores the previous behaviour.
---
 lisp/ob-ref.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/ob-ref.el b/lisp/ob-ref.el
index 5a3c8ba..251fa55 100644
--- a/lisp/ob-ref.el
+++ b/lisp/ob-ref.el
@@ -85,7 +85,9 @@ (defun org-babel-ref-parse (assignment)
       (cons (intern var)
 	    (let ((out (save-excursion
 			 (when org-babel-current-src-block-location
-			   (goto-char org-babel-current-src-block-location))
+			   (goto-char (if (markerp org-babel-current-src-block-location)
+					  (marker-position org-babel-current-src-block-location)
+					org-babel-current-src-block-location)))
 			 (org-babel-read ref))))
 	      (if (equal out ref)
 		  (if (string-match "^\".*\"$" ref)
-- 
1.8.4.2


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



Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Re: [BUG][PATCH] Marker points into wrong buffer

[Bastien]

Achim Gratz <Stromeko@nexgo.de> writes:

> Here's a better patch (I hope).

Feel free to install it, thanks!

-- 
 Bastien

Re: [BUG][PATCH] Marker points into wrong buffer

[Achim Gratz]

Bastien writes:
> Feel free to install it, thanks!


Applied to master.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Re: [BUG][PATCH] Marker points into wrong buffer

[Rasmus]

Achim Gratz <Stromeko@nexgo.de> writes:
> Introduced with r114064 on Emacs trunk.  Not sure if this is a bug in
> Org or Emacs, but the patch restores the previous behaviour.
> ---
>  lisp/ob-ref.el | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/ob-ref.el b/lisp/ob-ref.el
> index 5a3c8ba..251fa55 100644
> --- a/lisp/ob-ref.el
> +++ b/lisp/ob-ref.el
> @@ -85,7 +85,9 @@ (defun org-babel-ref-parse (assignment)
>        (cons (intern var)
>  	    (let ((out (save-excursion
>  			 (when org-babel-current-src-block-location
> -			   (goto-char org-babel-current-src-block-location))
> +			   (goto-char (if (markerp org-babel-current-src-block-location)
> +					  (marker-position org-babel-current-src-block-location)
> +					org-babel-current-src-block-location)))

Right.  I didn't know that marker ≠ char and that marker contains the
buffer.  markerp is the better test here.

–Rasmus

-- 
Summon the Mothership!

Re: [BUG] Marker points into wrong buffer

[Achim Gratz]

Rasmus writes:
> I bisected emacs-bzr to find the revision where it breaks for me
> (cf. attached test case).  It's this one:
>
>     committer: Dmitry Antipov <dmantipov@yandex.ru>

Please report this as an Emacs bug.  I can find no bug that this change
is fixing and it clearly alters (undocumented, AFAICS) behaviour,
specifically turning something into an error that the original code
silently allowed.  There may have been agood reason for this, but this
would then be NEWS-worthy since code using markers would have to be
checked for this.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Wavetables for the Terratec KOMPLEXER:
http://Synth.Stromeko.net/Downloads.html#KomplexerWaves