[drew.adams@oracle.com: customizing hl-line-face should reset global-hl-line-overlay to nil]

[drew.adams@oracle.com: customizing hl-line-face should reset global-hl-line-overlay to nil]

[Richard Stallman]

Would someone please DTRT and then ack?

------- Start of forwarded message -------
From: "Drew Adams" <drew.adams@oracle.com>
To: "Emacs-Pretest-Bug" <emacs-pretest-bug@gnu.org>
Date: Sat, 26 Aug 2006 18:09:25 -0700
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Subject: customizing hl-line-face should reset global-hl-line-overlay to nil
X-Spam-Status: No, score=0.0 required=5.0 tests=none autolearn=failed 
	version=3.0.4

Customizing `hl-line-face' has no immediate effect. You must save the
customization, quit Emacs, and restart Emacs, to see the effect.

This is because customizing it does not delete the overlay. It is
enough to set `global-hl-line-overlay' to nil, to see the face change,
but customization of `hl-line-face' does not do this. It should.


In GNU Emacs 22.0.50.1 (i386-msvc-nt5.1.2600)
 of 2006-07-19 on BOS-CTHEWLAP2
X server distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-msvc (12.00)'

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: ENU
  locale-coding-system: cp1252
  default-enable-multibyte-characters: t

Major mode: Help

Minor modes in effect:
  encoded-kbd-mode: t
  tooltip-mode: t
  tool-bar-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  auto-compression-mode: t
  line-number-mode: t
  view-mode: t

Recent input:
i t . M-q <backspace> , SPC i n s t e a d SPC o f SPC 
t h e SPC e n t i <backspace> <backspace> t i r e SPC 
l i n e SPC b e i n g SPC i n SPC a SPC s i n g l e 
SPC b o x . M-q <down-mouse-1> <mouse-1> C-c C-c <help-echo> 
<help-echo> y e s <return> <help-echo> <help-echo> 
<down-mouse-1> <down-mouse-1> <mouse-1> <help-echo> 
<help-echo> <help-echo> <help-echo> <help-echo> <help-echo> 
<help-echo> <help-echo> <help-echo> <help-echo> <help-echo> 
<help-echo> <help-echo> <help-echo> <help-echo> <help-echo> 
<help-echo> <help-echo> <help-echo> <help-echo> <help-echo> 
<menu-bar> <help-menu> <report-emacs-bug>

Recent messages:
Making completion list...done
Mark set
Type C-x 4 C-o RET to restore the other window.  
Auto-saving...done
Mark set
Undo!
Mark set
Auto-saving...done
Auto-saving...done
Sending...done



_______________________________________________
emacs-pretest-bug mailing list
emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug
------- End of forwarded message -------

Re: [drew.adams@oracle.com: customizing hl-line-face should reset global-hl-line-overlay to nil]

[martin rudalics]

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

> Customizing `hl-line-face' has no immediate effect. You must save the
> customization, quit Emacs, and restart Emacs, to see the effect.

Would the attached patch help?

[-- Attachment #2: hl-line.patch --]
[-- Type: text/plain, Size: 1539 bytes --]

*** hl-line.el.~1.31.~	Wed Feb  1 09:17:44 2006
--- hl-line.el	Mon Aug 28 15:58:58 2006
***************
*** 64,69 ****
--- 64,76 ----

  ;;; Code:

+ (defvar hl-line-overlay nil
+   "Overlay used by Hl-Line mode to highlight the current line.")
+ (make-variable-buffer-local 'hl-line-overlay)
+ 
+ (defvar global-hl-line-overlay nil
+   "Overlay used by Global-Hl-Line mode to highlight the current line.")
+ 
  (defgroup hl-line nil
    "Highlight the current line."
    :version "21.1"
***************
*** 72,77 ****
--- 79,93 ----
  (defcustom hl-line-face 'highlight
    "Face with which to highlight the current line."
    :type 'face
+   :require 'hl-line
+   :set (lambda (symbol value)
+          (set-default symbol value)
+ 	 (dolist (buffer (buffer-list))
+ 	   (with-current-buffer buffer
+ 	     (when hl-line-overlay
+ 	       (overlay-put hl-line-overlay 'face hl-line-face))))
+ 	 (when global-hl-line-overlay
+ 	   (overlay-put global-hl-line-overlay 'face hl-line-face)))
    :group 'hl-line)

  (defcustom hl-line-sticky-flag t
***************
*** 92,104 ****

  This variable is expected to be made buffer-local by modes.")

- (defvar hl-line-overlay nil
-   "Overlay used by Hl-Line mode to highlight the current line.")
- (make-variable-buffer-local 'hl-line-overlay)
- 
- (defvar global-hl-line-overlay nil
-   "Overlay used by Global-Hl-Line mode to highlight the current line.")
- 
  ;;;###autoload
  (define-minor-mode hl-line-mode
    "Buffer-local minor mode to highlight the line about point.
--- 108,113 ----

[-- 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: [drew.adams@oracle.com: customizing hl-line-face should reset global-hl-line-overlay to nil]

[Drew Adams]

    > Customizing `hl-line-face' has no immediate effect. You must save the
    > customization, quit Emacs, and restart Emacs, to see the effect.

    Would the attached patch help?

Looking at it (without trying it), I'm not sure, but I suppose so.

However, please see my other email about using a face instead of a user
option (variable). Wouldn't that also take care of this problem? That is, if
there is no face variable, and users change the face itself, then that would
be immediately reflected in the overlay, no?

BTW, what is this for: ":require 'hl-line"? (I'm not familiar with it.)

Re: [drew.adams@oracle.com: customizing hl-line-face should reset global-hl-line-overlay to nil]

[martin rudalics]

>     > Customizing `hl-line-face' has no immediate effect. You must save the
>     > customization, quit Emacs, and restart Emacs, to see the effect.
> 
>     Would the attached patch help?
> 
> Looking at it (without trying it), I'm not sure, but I suppose so.

How am I to know whether it works if you don't try it?

> However, please see my other email about using a face instead of a user
> option (variable). Wouldn't that also take care of this problem? That is, if
> there is no face variable, and users change the face itself, then that would
> be immediately reflected in the overlay, no?

It would.  But it would break other people's customizations as well.

> BTW, what is this for: ":require 'hl-line"? (I'm not familiar with it.)

Try without, maybe it's not needed.

RE: [drew.adams@oracle.com: customizing hl-line-face should reset global-hl-line-overlay to nil]

[Drew Adams]

    > However, please see my other email about using a face instead
    > of a user option (variable). Wouldn't that also take care of this
    > problem? That is, if there is no face variable, and users change
    > the face itself, then that would be immediately reflected in the
    > overlay, no?

    It would.  But it would break other people's customizations as well.

Do you really think that would affect a lot of people negatively? I don't. I
think that most users of hl-line (both of them ;-)) will appreciate having a
face to customize.

    From: Richard Stallman, Sent: Monday, August 28, 2006 2:52 AM
    Subject: Re: hl-line-face should be a face, not an option

        It would be better to define a face `hl-line' than to have option
        `hl-line-face', whose value must be an existing face.

    I agree.  Would someone please do that, and then ack?

I apologize for sending two different bug reports, since this one addresses
both problems - sorry you went to the trouble of trying to fix only the
other one. This is the better way to go, IMO.

Re: [drew.adams@oracle.com: customizing hl-line-face should reset global-hl-line-overlay to nil]

[Chong Yidong]

"Drew Adams" <drew.adams@oracle.com> writes:

>     > However, please see my other email about using a face instead
>     > of a user option (variable). Wouldn't that also take care of this
>     > problem? That is, if there is no face variable, and users change
>     > the face itself, then that would be immediately reflected in the
>     > overlay, no?
>
>     It would.  But it would break other people's customizations as well.
>
> Do you really think that would affect a lot of people negatively? I don't. I
> think that most users of hl-line (both of them ;-)) will appreciate having a
> face to customize.

I suspect the users who care about "having a face to customize" forms
is an even smaller subset than those that will get annoyed by having
their existing customizations break.  Note that hl-line-mode has been
in Emacs since 1999.

I think Martin's patch is correct.

RE: [drew.adams@oracle.com: customizing hl-line-face should reset global-hl-line-overlay to nil]

[Drew Adams]

    >     > However, please see my other email about using a face instead
    >     > of a user option (variable). Wouldn't that also take
    >     > care of this problem? That is, if there is no face variable,
    >     > and users change the face itself, then that would be
    >     > immediately reflected in the overlay, no?
    >
    >     It would.  But it would break other people's
    >     customizations as well.
    >
    > Do you really think that would affect a lot of people
    > negatively? I don't. I think that most users of hl-line
    > (both of them ;-)) will appreciate having a face to customize.

    I suspect the users who care about "having a face to customize" forms
    is an even smaller subset than those that will get annoyed by having
    their existing customizations break.  Note that hl-line-mode has been
    in Emacs since 1999.

    I think Martin's patch is correct.

I disagree, obviously. The users who might have customized that variable did
so precisely because the face that was its default value was inappropriate
for them. Some might even have created a new face to use, because no
existing face was appropriate. They are quite likely to be users who do care
about having a face to customize. This was a poor design from the beginning,
and it should be corrected, giving users a better way to customize the
feature, even at the minor cost of making them change their existing,
workaround customizations.

But if others agree with you that this is a big problem, then the face
should be added but the variable still left in place, with its default value
changed to the new face, instead of `highlight'. That (including Martin's
correction) makes the code much more complex than is necessary; I hope
others do not agree with you about this.

Re: [drew.adams@oracle.com: customizing hl-line-face should reset global-hl-line-overlay to nil]

[Richard Stallman]

    > Customizing `hl-line-face' has no immediate effect. You must save the
    > customization, quit Emacs, and restart Emacs, to see the effect.

    Would the attached patch help?

How about making hl-mode check occasionally whether the value
of hl-line-face has changed since the overlay was set up?
That would be cleaner because it would not depend on the user
to use the custom mechanism.  It would work with plain setq.

    However, please see my other email about using a face instead of a user
    option (variable). Wouldn't that also take care of this problem? That is, if
    there is no face variable, and users change the face itself, then that would
    be immediately reflected in the overlay, no?

Creating a special `hl-line' face for this seems like a good idea.
For compatibility, we can preserve the existing variable
`hl-line-face', and make its initial value `hl-line'.
That way, if users have customized by setting the variable,
their customizations will not break.

Re: [drew.adams@oracle.com: customizing hl-line-face should reset global-hl-line-overlay to nil]

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

> Creating a special `hl-line' face for this seems like a good idea.
> For compatibility, we can preserve the existing variable
> `hl-line-face', and make its initial value `hl-line'.
> That way, if users have customized by setting the variable,
> their customizations will not break.

I've implemented this and checked it in.

RE: [drew.adams@oracle.com: customizing hl-line-face should resetglobal-hl-line-overlay to nil]

[Drew Adams]

    > Creating a special `hl-line' face for this seems like a good idea.
    > For compatibility, we can preserve the existing variable
    > `hl-line-face', and make its initial value `hl-line'.
    > That way, if users have customized by setting the variable,
    > their customizations will not break.
    
    I've implemented this and checked it in.
    
Thx.