all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Customizing recentf
@ 2006-08-25 15:53 Michael Mauger
  2006-08-28  9:52 ` Richard Stallman
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Mauger @ 2006-08-25 15:53 UTC (permalink / raw)


I updated my w32 build two days ago to replace a late-June build (via a make 
bootstrap).  The first thing I noticed was that my recentf list of recently 
editted files was missing.  

I use Customize to enable and configure recentf.  I have the following in my 
custom-set-variables:

 '(recentf-mode t)
 '(recentf-save-file "~/.emacs-recentf")

Debugging the issue, it appears that recentf was being enabled before the 
recentf-save-file variable was set.  So it was reading the (non-existent) 
~/.recentf (thus an empty list of recently used files), but when I left Emacs 
it was saving to the file I had customized.

If I changed the recentf-mode line to:

 '(recentf-mode t nil (recentf))

it works as desired.  However, if I customize anything, the line reverts back 
to the original.  Obviously, getting rid of the save-file customization would 
work too.

I dug a little deeper, and saw that activation of minor modes under customize 
should be delayed to address just this issue, but the process of trying to 
figure out what went wrong began to make my brain go "owwwie".  

Anyone else seeing this?  I can provide additional info if it would be helpful.

This is GNU Emacs 22.0.50.1 (i386-mingw-nt5.1.2600)
 of 2006-08-23 on ASSHOLE1

  (Yes, that is the name of my machine ;-) )

-- Michael Mauger

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Customizing recentf
  2006-08-25 15:53 Customizing recentf Michael Mauger
@ 2006-08-28  9:52 ` Richard Stallman
  2006-08-28 15:02   ` Stefan Monnier
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Stallman @ 2006-08-28  9:52 UTC (permalink / raw)
  Cc: emacs-devel

Does the patch below make it work?

We made a change so that minor mode variables don't normally 
force loading of their files for customization purposes,
because minor mode variables in general don't need to do so.
This one appears to need it for specific reasons.
We can fix each such case in this manner.  Maybe that is
a suitable approach.  Does anyone see a problem with it?

*** recentf.el	03 Apr 2006 17:30:03 -0400	1.53
--- recentf.el	28 Aug 2006 04:35:41 -0400	
***************
*** 1324,1329 ****
--- 1324,1330 ----
    :global t
    :group 'recentf
    :keymap recentf-mode-map
+   :require recentf
    (unless (and recentf-mode (recentf-enabled-p))
      (if recentf-mode
          (progn

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Customizing recentf
  2006-08-28  9:52 ` Richard Stallman
@ 2006-08-28 15:02   ` Stefan Monnier
  2006-08-29 11:47     ` Richard Stallman
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2006-08-28 15:02 UTC (permalink / raw)
  Cc: Michael Mauger, emacs-devel

> Does the patch below make it work?
> We made a change so that minor mode variables don't normally 
> force loading of their files for customization purposes,
> because minor mode variables in general don't need to do so.
> This one appears to need it for specific reasons.
> We can fix each such case in this manner.  Maybe that is
> a suitable approach.  Does anyone see a problem with it?

I think we should first figure out why this one needs it.  It doesn't
seem particularly special.  There may simply be a bug elsewhere,


        Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Customizing recentf
  2006-08-28 15:02   ` Stefan Monnier
@ 2006-08-29 11:47     ` Richard Stallman
  2006-08-29 19:04       ` Stefan Monnier
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Stallman @ 2006-08-29 11:47 UTC (permalink / raw)
  Cc: mmaug, emacs-devel

    I think we should first figure out why this one needs it.  It doesn't
    seem particularly special.  There may simply be a bug elsewhere,

Well, mmaug already sent an explanation of why it fails.
Do you see a bug in the chain of events he described?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Customizing recentf
  2006-08-29 11:47     ` Richard Stallman
@ 2006-08-29 19:04       ` Stefan Monnier
  2006-08-29 23:09         ` Michael Mauger
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Monnier @ 2006-08-29 19:04 UTC (permalink / raw)
  Cc: mmaug, emacs-devel

>     I think we should first figure out why this one needs it.  It doesn't
>     seem particularly special.  There may simply be a bug elsewhere,

> Well, mmaug already sent an explanation of why it fails.
> Do you see a bug in the chain of events he described?

Not really, but the chain is incomplete.  See the part where he says "owwie":

>> I dug a little deeper, and saw that activation of minor modes under
>> customize should be delayed to address just this issue, but the process
>> of trying to  figure out what went wrong began to make my brain go
>> "owwwie".  


-- Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Customizing recentf
  2006-08-29 19:04       ` Stefan Monnier
@ 2006-08-29 23:09         ` Michael Mauger
  2006-08-30  0:32           ` Michael Mauger
                             ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Michael Mauger @ 2006-08-29 23:09 UTC (permalink / raw)


Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

> 
> >     I think we should first figure out why this one needs it.  It doesn't
> >     seem particularly special.  There may simply be a bug elsewhere,
> 
> > Well, mmaug already sent an explanation of why it fails.
> > Do you see a bug in the chain of events he described?
> 
> Not really, but the chain is incomplete.  See the part where he says "owwie":
> 
> >> I dug a little deeper, and saw that activation of minor modes under
> >> customize should be delayed to address just this issue, but the process
> >> of trying to  figure out what went wrong began to make my brain go
> >> "owwwie".  
> 

Okay, I think I've figured out what is happening, not sure how best to fix it...

This is probably not a problem with just recentf.  I'm seeing a problem only 
because this is a minor mode which hasn't been loaded yet and there are mode 
related settings that are alphabetically sorted after the minor mode variable 
name.  On top of that, this particular mode uses the later variable when it 
starts.  My guess is that there are other minor modes out there with similar 
problems but which may be far more subtle and finding them is a challenge.

Some background:

* the define-minor-mode of recentf-mode is Autoloaded
* minor mode variables or variables having a require list (the 4th element in 
the custom-set-varaiables entry) are moved to the end of the list to be set.

RMS solution was to force the creation of the 4th element in the custom-set-
variables entry by adding the :require recentf property to the define-minor-
mode command.  When custom is next updated, "nil (recentf)" is added to the 
entry automatically.  This fixes recentf but doesn't address the underlying 
problem.

The issue is that when custom-set-variable checks to see if a symbol is a minor-
mode, it looks at the custom-set property which isn't set until the module is 
loaded.  Once the module has been loaded, custom properly detects that recentf-
mode is a minor mode variable.

If I look at the plist of recentf-mode prior to the custom-set-variable, it 
looks like:

  (variable-documentation 1424541 
   custom-autoload t 
   custom-loads ("recentf"))

The custom-autoload and custom-loads properties are used once the c-s-v entries 
have been sorted, what I'm wondering is, if processing these before the sort 
makes sense, so that the optional minor mode modules get loaded before checking 
their properties?  

Here's an attempt (basically I moved the code to autoload out of the main loop 
and put it before the sort).  This did fix the problem for me and would seem to 
fix the problem for other, more subtle, situations.  I didn't move the code to 
handle explicit requires in the c-s-v entries since I figured that should be 
reserved for very special situations.  This patch should allow us to remove 
most of the :require settings in autoloaded minor modes.

*** emacs/lisp/custom.el.orig   Wed Aug 23 20:21:04 2006
--- emacs/lisp/custom.el        Tue Aug 29 18:56:36 2006
***************
*** 874,879 ****
--- 874,891 ----
  EXP itself is saved unevaluated as SYMBOL property `saved-value' and
  in SYMBOL's list property `theme-value' \(using `custom-push-theme')."
    (custom-check-theme theme)
+ 
+   ;; Load any modules referred to by symbols in the set-variable list
+   ;; This allows us to detect minor modes even if they hadn't been
+   ;; loaded yet.
+   (dolist (entry args)
+     (let* ((symbol (indirect-variable (nth 0 entry))))
+       (unless (or (get symbol 'standard-value)
+                 (memq (get symbol 'custom-autoload) '(nil noset)))
+       ;; This symbol needs to be autoloaded, even just for a `set'.
+       (custom-load-symbol symbol))))
+ 
+   ;; Move minor modes and variables with explicit requires to the end
    (setq args
        (sort args
              (lambda (a1 a2)
***************
*** 904,913 ****
            (when requests
              (put symbol 'custom-requests requests)
              (mapc 'require requests))
-             (unless (or (get symbol 'standard-value)
-                         (memq (get symbol 'custom-autoload) '(nil noset)))
-               ;; This symbol needs to be autoloaded, even just for a `set'.
-               (custom-load-symbol symbol))
            (setq set (or (get symbol 'custom-set) 'custom-set-default))
            (put symbol 'saved-value (list value))
            (put symbol 'saved-variable-comment comment)
--- 916,921 ----

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Customizing recentf
  2006-08-29 23:09         ` Michael Mauger
@ 2006-08-30  0:32           ` Michael Mauger
  2006-08-30 17:17           ` Stefan Monnier
                             ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Michael Mauger @ 2006-08-30  0:32 UTC (permalink / raw)


Michael Mauger writes:

> Here's an attempt ...

FYI, in my own configuration, it caused the following minor modes to be 
recognized and moved to the end of the set-variables list as opposed to without 
the patch:

*  show-paren-mode
*  global-hl-line-mode
*  global-auto-revert-mode
*  display-time-mode
*  auto-insert-mode

Now, none of these minor modes may exhibit the same issue as recentf re: the 
sort order of mode related variables, but you never know...

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Customizing recentf
  2006-08-29 23:09         ` Michael Mauger
  2006-08-30  0:32           ` Michael Mauger
@ 2006-08-30 17:17           ` Stefan Monnier
  2006-08-30 17:58           ` Richard Stallman
  2006-08-31 17:11           ` Stefan Monnier
  3 siblings, 0 replies; 10+ messages in thread
From: Stefan Monnier @ 2006-08-30 17:17 UTC (permalink / raw)
  Cc: emacs-devel

> The custom-autoload and custom-loads properties are used once the c-s-v
> entries  have been sorted, what I'm wondering is, if processing these
> before the sort  makes sense, so that the optional minor mode modules get
> loaded before checking  their properties?  

That would make perfect sense, yes.

> Here's an attempt (basically I moved the code to autoload out of the main
> loop  and put it before the sort).  This did fix the problem for me and
> would seem to  fix the problem for other, more subtle, situations.
> I didn't move the code to  handle explicit requires in the c-s-v entries
> since I figured that should be  reserved for very special situations.
> This patch should allow us to remove  most of the :require settings in
> autoloaded minor modes.

Looks good, thank you.


        Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Customizing recentf
  2006-08-29 23:09         ` Michael Mauger
  2006-08-30  0:32           ` Michael Mauger
  2006-08-30 17:17           ` Stefan Monnier
@ 2006-08-30 17:58           ` Richard Stallman
  2006-08-31 17:11           ` Stefan Monnier
  3 siblings, 0 replies; 10+ messages in thread
From: Richard Stallman @ 2006-08-30 17:58 UTC (permalink / raw)
  Cc: emacs-devel

I think you have found a good fix.  Thank you.
Would someone please install it in a few days
if no problem has been found?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: Customizing recentf
  2006-08-29 23:09         ` Michael Mauger
                             ` (2 preceding siblings ...)
  2006-08-30 17:58           ` Richard Stallman
@ 2006-08-31 17:11           ` Stefan Monnier
  3 siblings, 0 replies; 10+ messages in thread
From: Stefan Monnier @ 2006-08-31 17:11 UTC (permalink / raw)
  Cc: emacs-devel

> Here's an attempt (basically I moved the code to autoload out of the main
> loop  and put it before the sort).  This did fix the problem for me and
> would seem to  fix the problem for other, more subtle, situations.
> I didn't move the code to  handle explicit requires in the c-s-v entries
> since I figured that should be  reserved for very special situations.
> This patch should allow us to remove  most of the :require settings in
> autoloaded minor modes.

Installed,


        Stefan

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2006-08-31 17:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-25 15:53 Customizing recentf Michael Mauger
2006-08-28  9:52 ` Richard Stallman
2006-08-28 15:02   ` Stefan Monnier
2006-08-29 11:47     ` Richard Stallman
2006-08-29 19:04       ` Stefan Monnier
2006-08-29 23:09         ` Michael Mauger
2006-08-30  0:32           ` Michael Mauger
2006-08-30 17:17           ` Stefan Monnier
2006-08-30 17:58           ` Richard Stallman
2006-08-31 17:11           ` Stefan Monnier

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.