Subject: Re: bug#5809: 23.1.94; cross-reference by anchor yields in accurate position

From: Juri Linkov <[email-masked]>
Date: Mon, 05 Apr 2010 19:50:52 +0300

>> We could change the background color of the overlay's after-string
>> to look like the header line (grey background) so users will expect
>> that only mouse clicks should work on grey areas.
>
> I'd rather not change this part of the visual appearance, but maybe it's
> just my personal preference.  I think this decision should be taken
> with the understanding that we will want to install a real-fix in
> Emacs-24 so that we can click with the keyboard and copy&paste
> the breadcrumbs and that we won't want to revert the visual appearance
> at that point (people get used to visual appearances).

I think both types of navigation links (breadcrumbs and up/next/prev)
should be treated equally.  If we'll implement clicking with the keyboard
and copy&paste in Emacs-24, it would be natural to apply this to the
up/next/prev links as well and change their visual appearance.

Or maybe to leave breadcrumbs and navigation links in the (multi-line?)
header line will be better because they don't scroll and stay on top.

In any case, it's important that the visual appearance should match the
user's expectation.  When the visual appearance of breadcrumbs is
the same as for the rest text of the Info buffer, users will be tempted
to use the keyboard on breadcrumbs.

> If we want to use this header-line appearance, couldn't we use something
> like font-lock-append-text-property rather than apply `header-line'
> bit-by-bit (and worse yet: in different ways for different parts).

Done in the patch below.

>> @@ -4260,7 +4288,10 @@ (defun Info-fontify-node ()
>>              ;; that is in the header, if it is just part.
>>              (cond
>>               ((> Info-breadcrumbs-depth 0)
>> -              (put-text-property (point-min) (1+ header-end) 'invisible t))
>> +              (put-text-property (point-min) (1+ header-end) 'invisible t)
>> +	      (overlay-put
>> +	       (make-overlay header-end (1+ header-end))
>> +	       'after-string (Info-breadcrumbs)))
>>               ((not (bobp))
>>                ;; Hide the punctuation at the end, too.
>>                (skip-chars-backward " \t,")
>
> Why is the `overlay-put' at a different place than the
> former Info-insert-breadcrumbs?

The overlay doesn't correctly interact with the `invisible' text property.
However, we can put 'invisible on the overlay instead of the text property:

    (let ((ov (make-overlay (point-min) (1+ header-end))))
      (overlay-put ov 'invisible t)
      (overlay-put ov 'after-string (Info-breadcrumbs))
      (overlay-put ov 'evaporate t))

> PS: the rest of the patch looks OK, so if you can fix the above
> feel free to install it.

I'll give this patch more testing before installing:

=== modified file 'lisp/info.el'
--- lisp/info.el	2010-03-03 19:23:20 +0000
+++ lisp/info.el	2010-04-05 16:48:03 +0000
@@ -1053,8 +1053,8 @@ (defun Info-find-node-2 (filename nodena
 	    (Info-select-node)
 	    (goto-char (point-min))
 	    (forward-line 1)		       ; skip header line
-	    (when (> Info-breadcrumbs-depth 0) ; skip breadcrumbs line
-	      (forward-line 1))
+	    ;; (when (> Info-breadcrumbs-depth 0) ; skip breadcrumbs line
+	    ;;   (forward-line 1))
 
 	    (cond (anchorpos
                    (let ((new-history (list Info-current-file
@@ -3551,6 +3551,19 @@ (defun Info-try-follow-nearest-node (&op
      ((setq node (Info-get-token (point) "Prev: " "Prev: \\([^,\n\t]*\\)"))
       (Info-goto-node node fork)))
     node))
+
+(defun Info-mouse-follow-link (click)
+  "Follow a link under point."
+  (interactive "e")
+  (let* ((position (event-start click))
+	 (posn-string (and position (posn-string position)))
+	 (string (car-safe posn-string))
+	 (string-pos (cdr-safe posn-string))
+	 (link-args (and string string-pos
+			 (get-text-property string-pos 'link-args string))))
+    (when link-args
+      (Info-goto-node link-args))))
+
 
 (defvar Info-mode-map
   (let ((map (make-keymap)))
@@ -4141,11 +4154,22 @@ (defvar Info-up-link-keymap
     keymap)
   "Keymap to put on the Up link in the text or the header line.")
 
-(defun Info-insert-breadcrumbs ()
+(defvar Info-link-keymap
+  (let ((keymap (make-sparse-keymap)))
+    (define-key keymap [header-line mouse-1] 'Info-mouse-follow-link)
+    (define-key keymap [header-line mouse-2] 'Info-mouse-follow-link)
+    (define-key keymap [header-line down-mouse-1] 'ignore)
+    (define-key keymap [mouse-2] 'Info-mouse-follow-link)
+    (define-key keymap [follow-link] 'mouse-face)
+    keymap)
+  "Keymap to put on the link in the text or the header line.")
+
+(defun Info-breadcrumbs ()
   (let ((nodes (Info-toc-nodes Info-current-file))
 	(node Info-current-node)
         (crumbs ())
-        (depth Info-breadcrumbs-depth))
+        (depth Info-breadcrumbs-depth)
+	line)
 
     ;; Get ancestors from the cached parent-children node info
     (while (and (not (equal "Top" node)) (> depth 0))
@@ -4172,15 +4196,26 @@ (defun Info-insert-breadcrumbs ()
 			     (file-name-nondirectory Info-current-file)
 			   ;; Some legacy code can still use a symbol.
 			   Info-current-file)))))
-	  (insert (if (bolp) "" " > ")
-		  (cond
-		   ((null node) "...")
-		   ((equal node Info-current-node)
-		    ;; No point linking to ourselves.
-		    (propertize text 'font-lock-face 'info-header-node))
-		   (t
-		    (concat "*Note " text "::"))))))
-      (insert "\n"))))
+	  (setq line (concat
+		      line
+		      (if (null line) "" " > ")
+		      (cond
+		       ((null node) "...")
+		       ((equal node Info-current-node)
+			;; No point linking to ourselves.
+			(propertize text 'font-lock-face 'info-header-node))
+		       (t
+			(propertize text
+				    'mouse-face 'highlight
+				    'font-lock-face 'info-header-xref
+				    'help-echo "mouse-2: Go to node"
+				    'keymap Info-link-keymap
+				    'link-args text)
+			))))))
+      (setq line (concat line "\n")))
+    (font-lock-append-text-property 0 (length line)
+				    'font-lock-face 'header-line line)
+    line))
 
 (defun Info-fontify-node ()
   "Fontify the node."
@@ -4227,8 +4262,8 @@ (defun Info-fontify-node ()
 		((string-equal (downcase tag) "next") Info-next-link-keymap)
 		((string-equal (downcase tag) "up"  ) Info-up-link-keymap))))))
 
-        (when (> Info-breadcrumbs-depth 0)
-          (Info-insert-breadcrumbs))
+        ;; (when (> Info-breadcrumbs-depth 0)
+        ;;   (insert (Info-breadcrumbs)))
 
         ;; Treat header line.
         (when Info-use-header-line
@@ -4260,7 +4295,10 @@ (defun Info-fontify-node ()
             ;; that is in the header, if it is just part.
             (cond
              ((> Info-breadcrumbs-depth 0)
-              (put-text-property (point-min) (1+ header-end) 'invisible t))
+	      (let ((ov (make-overlay (point-min) (1+ header-end))))
+		(overlay-put ov 'invisible t)
+		(overlay-put ov 'after-string (Info-breadcrumbs))
+		(overlay-put ov 'evaporate t)))
              ((not (bobp))
               ;; Hide the punctuation at the end, too.
               (skip-chars-backward " \t,")

-- 
Juri Linkov
http://www.jurta.org/emacs/