Subject: bug#12507: Option `(bookmark-)version-control': Use :tag so docstring matches menu

From: "Drew Adams" <[email-masked]>
Date: Sun, 30 Sep 2012 21:06:00 -0700

Ouch!

You fixed bug #12309, Karl (thanks; looks good).  But you closed bug #12507
instead.

Please reopen #12507 and correct the subject lines etc. of the mails if needed,
so that things are put right again.  It is #12309 that should be closed, not
@12507.

To answer your questions:

`other' in a `choice' just means any value other than the other choices listed.
If the user picks the `other' choice interactively then the value given (`t'
here) is used.  E.g., if you set the variable to 99999 (outside of Customize)
then it will act just like a value of `t'.

The last sentence of the original report just meant to please at least use a
:tag for the `other' choice.  It is the one where a :tag is most important for
clarity.  But you've added :tag for each of them, which is even better.

Thx - Drew

P.S. FWIW, here is what I've been using for this defcustom.  It's almost the
same as what you have.

"Whether to make numbered backups of your bookmarking files.
The option can have value `nospecial', `t', `nil', or `never' .  Value
`nospecial' means to use the `version-control' value.  The others have
the same meanings as for option `version-control'.

Use value `t' if your bookmarks are important to you.  Consider also
using numeric backups.  See also nodes `Backup Names' and `Backup
Deletion' in the Emacs manual."

  :type '(choice :tag "When to make numbered backups"
          (const :tag "Use value of option `version-control'" nospecial)
          (const :tag "Never"                                 never)
          (const :tag "If existing already"                   nil)
          (other :tag "Always"                                t))

> From: Karl Fogel Sent: Sunday, September 30, 2012 8:38 PM
> To: [email-masked]
> Subject: bug#12507: Option `(bookmark-)version-control': Use 
> :tag so docstring matches menu
> 
> Fixed (see below), but please review.  
> 
> I don't fully understand the whole customization system 
> because I never
> use it myself (I just read doc strings and set variables directly in
> Elisp), so I don't quite get what `other' mean if used instead of
> `const', and I didn't fully understand the last paragraph of the
> original bug report.  I looked in the Info pages, but they didn't
> clarify much about this.
> 
> -Karl
> 
> Revision info:
> 
>   revno: 110305
>   revision-id: [email-masked]
>   parent: [email-masked]
>   committer: Karl Fogel <[email-masked]>
>   branch nick: trunk
>   timestamp: Sun 2012-09-30 22:32:06 -0500
>   message:
>     * lisp/bookmark.el (bookmark-version-control): Give tags in the
>       :type choices (Bug#12309), and improve doc string.
> 
> Diff:
> 
>   === modified file 'lisp/ChangeLog'
>   --- lisp/ChangeLog	2012-10-01 02:07:14 +0000
>   +++ lisp/ChangeLog	2012-10-01 03:31:41 +0000
>   @@ -1,3 +1,8 @@
>   +2012-10-01  Karl Fogel  <[email-masked]>
>   +
>   +	* bookmark.el (bookmark-version-control): Give tags in the
>   +	:type choices (Bug#12309), and improve doc string.
>   +
>    2012-10-01  Paul Eggert  <[email-masked]>
>    
>    	Revert the FOLLOW-SYMLINKS change for file-attributes.
>   
>   === modified file 'lisp/bookmark.el'
>   --- lisp/bookmark.el	2012-09-25 04:13:02 +0000
>   +++ lisp/bookmark.el	2012-10-01 03:32:18 +0000
>   @@ -99,12 +99,14 @@
>    
>    (defcustom bookmark-version-control 'nospecial
>      "Whether or not to make numbered backups of the bookmark file.
>   -It can have four values: t, nil, `never', and `nospecial'.
>   +It can have four values: t, nil, `never', or `nospecial'.
>    The first three have the same meaning that they do for the
>   -variable `version-control', and the final value 
> `nospecial' means just
>   -use the value of `version-control'."
>   -  :type '(choice (const nil) (const never) (const nospecial)
>   -		 (other t))
>   +variable `version-control'; the value `nospecial' (the 
> default) means
>   +just use the value of `version-control'."
>   +  :type '(choice (const :tag "If existing" nil)
>   +                 (const :tag "Never" never)
>   +                 (const :tag "Use the value of 
> `version-control'" nospecial)
>   +                 (const :tag "Always" t))
>      :group 'bookmark)