bug#16755: 24.3; customize-save-variable saves the wrong value

bug#16755: 24.3; customize-save-variable saves the wrong value

[Sean Peters]

Bug:

customize-save-variable saves the wrong value if custom-set changes
the value.

Example:

(defcustom emacspeak-feeds
  '(
    ("Wired News" "http://www.wired.com/news_drop/netcenter/netcenter.rdf"  rss)
    ("BBC Podcast Directory" "http://www.bbc.co.uk/podcasts.opml" opml)
    ("BBC News"  "http://www.bbc.co.uk/syndication/feeds/news/ukfs_news/front_page/rss091.xml"  rss)
    ("CNet Tech News"  "http://feeds.feedburner.com/cnet/tcoc"  rss)
    )
  "Table of RSS feeds."
  :type '(repeat
          (list :tag "Feed"
                (string :tag "Title")
                (string :tag "URI")
                (choice :tag "Type"
                        (const :tag "RSS" rss)
                        (const :tag "opml" opml)
                        (const :tag "Atom" atom))))
  :initialize  'custom-initialize-reset
  :set
  #'(lambda (sym val)
      (set-default
       sym
       (sort val #'(lambda (a b)
                     (string-lessp (first a) (first b))))))
  :group 'emacspeak-feeds)

(customize-save-variable 'emacspeak-feeds (push '("z" "z" 'rss) emacspeak-feeds))

The value that gets saved is just '("z" "z" (quote rss)) instead of
the existing list with this on the end.

Cause:

These two lines from customize-save-variable show the problem:
  (funcall (or (get variable 'custom-set) 'set-default) variable value)
  (put variable 'saved-value (list (custom-quote value)))

During the call of custom-set, value gets destructively sorted.  So
when value is used for setting saved-value it is missing the part of
the sorted list of settings before the newly added feed.

Fix:

I think customize-save-variable should use (default-value variable)
instead of value in all the places it currently uses value after the
call to custom-set.

Also, customize-set-variable may have a similar problem.

Sean

-- 

bug#16755: 24.3; customize-save-variable saves the wrong value

[Stefan Monnier]

> I think customize-save-variable should use (default-value variable)
> instead of value in all the places it currently uses value after the
> call to custom-set.

But that can't work: a "custom variable" may not even have
a `default-value'.  The :get and :set are free to completely change
between the world of "value as seen by Custom and the end user" and "the
world of Elisp" where that corresponding value can be represented in
completely different ways.

This is rarely used, admittedly, but attempts at providing support for
Customizing keymaps or things like `load-path' rely on this feature.

So I think the better course is to document that the ":set" function should
not modify its `val' argument destructively.


        Stefan

bug#16755: 24.3; customize-save-variable saves the wrong value

[Juanma Barranquero]

On Fri, Feb 14, 2014 at 7:23 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

> So I think the better course is to document that the ":set" function should
> not modify its `val' argument destructively.

Done.