Subject: bug#26694: preview-at-point fails on very long lines with "End of buffer"

From: Ikumi Keita <[email-masked]>
Date: Sun, 28 May 2017 22:12:02 +0900

[ Dropped Frank from the recipients. ]

Hi Mosè and all,

> I think you're right, a good solution would be to add one more
> argument to `TeX-region-create' with the adjusted line-col
> information.  However, in order to avoid changing the signature of the
> function I decided to apply a work around: the point is moved only
> when `TeX-source-correlate-mode' is non nil (because point is moved
> only to make forward/inverse search work) and this variable is
> let-bound to nil when running the preview (forward/inverse search is
> not needed here).

> I dropped a few comments about this work-around in the code, in case
> someone want to improve the fix, maybe as suggested above.  In any
> case, I'm closing this ticket as the issue should be fixed.

I looked into this problem and would like to share my thoughts about
this.

(1) Some problems of the essentially same origin still remain.
   (a) Open Frank's sample file and enable TeX-source-correlate-mode.
   (b) Put the region over "$x$".
   (c) Type C-c C-p C-r.  "End of buffer" error again.  C-c C-r gives
       the same error, too.

    Here is the relavant codes Mosè modified in tex-buf.el:
	(when TeX-source-correlate-mode
	    (let ((line-col (with-current-buffer orig-buffer
			   (cons (TeX-line-number-at-pos)
				 (current-column)))))
	   (goto-char (point-min))
	   (forward-line (1- (abs (- header-offset (car line-col)))))
	   (forward-char (cdr line-col))))
    It is this `forward-char' that raises the error.  The number
    returned by `(current-column)' is not correct when, in original
    buffer, the region starts in the middle of a line and the point is
    on the first line of the region.  If a relatively small region has
    no newline in it and is put on the rightward enough of a very long
    line, the returned value is too large for `forward-char'.

(2) We can find a function `TeX-region-update-point', which does a
    similar job but without adjustment of the column.  I suppose we can
    unify them.

Considering that, in addition to (1) and (2), update of the position of
the point in _region_.tex is only necessary when we invoke viewer, call
to `TeX-region-update-point' can be summarized in `TeX-command' with
test of `(string= name "View")'.

I made a tentative patch along these idea and attached it to this
message.  I stopped using `current-column' and use difference betewwn
point and max{beginning of line, beginning of region}.

(3) It seems that the feature of forward and inverse search for region
    file is not mature yet.  If I understand correctly, it works only
    with evince, atril and pdf-tools as viewer, right?  The other
    viewers in `TeX-view-program-list-builtin' receive the file name and
    line number with "%b" and "%n" expanders, but these are not expanded
    to suitable value when using region file.
    Maybe we should add other expanders which work even with region
    file.

(4) Using `current-column' might not be suitable even when we use evince
    or atril.  The function `TeX-evince-sync-view-1' passes required
    parameters to the viewers like this:
	   (list :struct :int32 (TeX-line-number-at-pos)
		 :int32 (1+ (current-column)))
    Most of CJK letters count as occuping 2 columns, so the column
    number is not equal to the number of the characters counting from
    the beginning of a line.   (I don't know what is the correct
    "column" parameters for evince and atril (number of letters?
    bytes in UTF8? or other?), so don't know how we should change this
    part.)

Best,
Ikumi Keita

diff --git a/tex-buf.el b/tex-buf.el
--- a/tex-buf.el
+++ b/tex-buf.el
@@ -492,6 +492,11 @@
 	 (setq TeX-current-process-region-p t))
 	((eq file #'TeX-master-file)
 	 (setq TeX-current-process-region-p nil)))
+
+  ;; When we're operating on a region, we need to update the position
+  ;; of point in the region file so that forward search works.
+  (if (string= name "View") (TeX-region-update-point))
+
   (let ((command (TeX-command-expand (nth 1 (assoc name TeX-command-list))
 				     file))
 	(hook (nth 2 (assoc name TeX-command-list)))
@@ -899,17 +904,21 @@
 the current buffer.
 
 Does nothing in case the last command hasn't operated on the
-region."
-  (when TeX-current-process-region-p
+region or `TeX-source-correlate-mode' is disabled."
+  (when (and TeX-current-process-region-p TeX-source-correlate-mode))
     (let ((region-buf (get-file-buffer (TeX-region-file t)))
-	  (current-line (TeX-line-number-at-pos)))
+	  (orig-line (TeX-current-offset))
+	  (pos-in-line (- (point) (max (line-beginning-position)
+				       (or TeX-command-region-begin
+					   (region-beginning))))))
       (when region-buf
 	(with-current-buffer region-buf
 	  (goto-char (point-min))
-	  (when (re-search-forward "!offset(\\(-?[0-9]+\\)")
+	  (when (re-search-forward "!offset(\\(-?[0-9]+\\)" nil t)
 	    (let ((offset (string-to-number (match-string 1))))
 	      (goto-char (point-min))
-	      (forward-line (- current-line (1+ offset))))))))))
+	      (forward-line (- orig-line offset))
+	      (forward-char pos-in-line))))))))
 
 (defun TeX-view ()
   "Start a viewer without confirmation.
@@ -918,11 +927,7 @@
   (interactive)
   (let ((output-file (TeX-active-master (TeX-output-extension))))
     (if (file-exists-p output-file)
-	(progn
-	  ;; When we're operating on a region, we need to update the position
-	  ;; of point in the region file so that forward search works.
-	  (TeX-region-update-point)
-	  (TeX-command "View" 'TeX-active-master 0))
+	(TeX-command "View" 'TeX-active-master 0)
       (message "Output file %S does not exist." output-file))))
 
 (defun TeX-output-style-check (styles)
@@ -2140,17 +2145,6 @@
 		") }\n"
 		trailer)
 	(setq TeX-region-orig-buffer orig-buffer)
-	;; Position point at the line/col that corresponds to point's line in
-	;; orig-buffer in order to make forward search work.  Move point only
-	;; when `TeX-source-correlate-mode' is non-nil, in order to work around
-	;; bug#26694.
-	(when TeX-source-correlate-mode
-	    (let ((line-col (with-current-buffer orig-buffer
-			   (cons (TeX-line-number-at-pos)
-				 (current-column)))))
-	   (goto-char (point-min))
-	   (forward-line (1- (abs (- header-offset (car line-col)))))
-	   (forward-char (cdr line-col))))
 	(run-hooks 'TeX-region-hook)
 	(if (string-equal (buffer-string) original-content)
 	    (set-buffer-modified-p nil)