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

From: Stefan Monnier <[email-masked]>
Date: Sun, 04 Apr 2010 22:06:31 -0400

> 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).

> -	    (when (> Info-breadcrumbs-depth 0) ; skip breadcrumbs line
> -	      (forward-line 1))

I'd keep it commented out, since it may be useful again when we revert
to using actual text rather than an after-string.
 
> +	  (setq line (concat
> +		      line
> +		      (if (null line) "" (propertize " > " 'face 'header-line))
> +		      (cond
> +		       ((null node) (propertize "..." 'face 'header-line))
> +		       ((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)
> +			))))))

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).

I.e. the changes to info-header-xref and info-header-node faces earlier
in the patch are not a good idea (think of people who changed those
faces, for example).

> @@ -4227,9 +4258,6 @@ (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))
> -
>          ;; Treat header line.
>          (when Info-use-header-line
>            (goto-char (point-min))
> @@ -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?

        Stefan

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