unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#44598: [PATCH] Do not show obsolete options in customize
       [not found] ` <83lfh743j8.fsf@gnu.org>
@ 2020-11-12 20:56   ` Stefan Kangas
  2020-11-12 21:11     ` Basil L. Contovounesios
                       ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Stefan Kangas @ 2020-11-12 20:56 UTC (permalink / raw)
  To: 44598

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

Severity: wishlist

In a recent discussion on emacs-devel,[1]

Eli Zaretskii <eliz@gnu.org> wrote:

>> The obsolete options use a different face.  However, it's not obvious
>> that this is the meaning of that face.
>
> IMO, we shouldn't show obsolete options at all in a Custom buffer, for
> the same reason why we remove them from the manuals.

How about the attached patch?

[-- Attachment #2: Type: text/plain, Size: 76 bytes --]


Footnotes:
[1]  https://lists.gnu.org/r/emacs-devel/2020-09/msg01620.html


[-- Attachment #3: 0001-Do-not-show-obsolete-options-in-customize.patch --]
[-- Type: text/x-diff, Size: 7533 bytes --]

From 66221f1d0f7ee4f2af0d6c65fe956cce711b48e2 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Sat, 24 Oct 2020 19:44:20 +0200
Subject: [PATCH] Do not show obsolete options in customize

* lisp/cus-edit.el (custom-variable-obsolete): Delete face.
(custom--filter-obsolete-variables): New defun.
(custom-variable-documentation, custom-variable-value-create)
(custom-group-value-create): Do not show obsolete user options.
* test/lisp/cus-edit-tests.el: New file.
---
 etc/NEWS                    |  4 +++
 lisp/cus-edit.el            | 61 ++++++++++++-------------------------
 test/lisp/cus-edit-tests.el | 46 ++++++++++++++++++++++++++++
 3 files changed, 69 insertions(+), 42 deletions(-)
 create mode 100644 test/lisp/cus-edit-tests.el

diff --git a/etc/NEWS b/etc/NEWS
index f21c4cb02c..7d064c0c11 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -706,6 +706,10 @@ This file was a compatibility kludge which is no longer needed.
 To revert to the previous behavior,
 '(setq lisp-indent-function 'lisp-indent-function)' from 'lisp-mode-hook'.
 
+** Customize
+
+*** Customize will no longer show obsolete user options.
+
 ** Edebug
 
 +++
diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index d1077d367d..b249b7ef42 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -2505,18 +2505,6 @@ custom-comment-invisible-p
 
 ;;; The `custom-variable' Widget.
 
-(defface custom-variable-obsolete
-  '((((class color) (background dark))
-     :foreground "light blue")
-    (((min-colors 88) (class color) (background light))
-     :foreground "blue1")
-    (((class color) (background light))
-     :foreground "blue")
-    (t :slant italic))
-  "Face used for obsolete variables."
-  :version "27.1"
-  :group 'custom-faces)
-
 (defface custom-variable-tag
   '((((class color) (background dark))
      :foreground "light blue" :weight bold)
@@ -2544,7 +2532,7 @@ custom-variable-documentation
 Normally just return the docstring.  But if VARIABLE automatically
 becomes buffer local when set, append a message to that effect.
 Also append any obsolescence information."
-  (format "%s%s%s" (documentation-property variable 'variable-documentation t)
+  (format "%s%s" (documentation-property variable 'variable-documentation t)
 	  (if (and (local-variable-if-set-p variable)
 		   (or (not (local-variable-p variable))
 		       (with-temp-buffer
@@ -2552,21 +2540,7 @@ custom-variable-documentation
 	      "\n
 This variable automatically becomes buffer-local when set outside Custom.
 However, setting it through Custom sets the default value."
-	    "")
-	  ;; This duplicates some code from describe-variable.
-	  ;; TODO extract to separate utility function?
-	  (let* ((obsolete (get variable 'byte-obsolete-variable))
-		 (use (car obsolete)))
-	    (if obsolete
-		(concat "\n
-This variable is obsolete"
-			(if (nth 2 obsolete)
-			    (format " since %s" (nth 2 obsolete)))
-			(cond ((stringp use) (concat ";\n" use))
-			      (use (format-message ";\nuse `%s' instead."
-						   (car obsolete)))
-			      (t ".")))
-	      ""))))
+            "")))
 
 (define-widget 'custom-variable 'custom
   "A widget for displaying a Custom variable.
@@ -2650,8 +2624,7 @@ custom-variable-value-create
 	 (state (or (widget-get widget :custom-state)
 		    (if (memq (custom-variable-state symbol value)
 			      (widget-get widget :hidden-states))
-			'hidden)))
-	 (obsolete (get symbol 'byte-obsolete-variable)))
+			'hidden))))
 
     ;; If we don't know the state, see if we need to edit it in lisp form.
     (unless state
@@ -2684,9 +2657,7 @@ custom-variable-value-create
 	   (push (widget-create-child-and-convert
 		  widget 'item
 		  :format "%{%t%} "
-		  :sample-face (if obsolete
-				   'custom-variable-obsolete
-				 'custom-variable-tag)
+		  :sample-face 'custom-variable-tag
 		  :tag tag
 		  :parent widget)
 		 buttons))
@@ -2744,9 +2715,7 @@ custom-variable-value-create
 		    :help-echo "Change value of this option."
 		    :mouse-down-action 'custom-tag-mouse-down-action
 		    :button-face 'custom-variable-button
-		    :sample-face (if obsolete
-				     'custom-variable-obsolete
-				   'custom-variable-tag)
+		    :sample-face 'custom-variable-tag
 		    :tag tag)
 		   buttons)
 	     (push (widget-create-child-and-convert
@@ -4232,6 +4201,13 @@ custom-group-visibility-create
 	(insert "--------")))
   (widget-default-create widget))
 
+(defun custom--filter-obsolete-variables (items)
+  "Filter obsolete variables from ITEMS."
+  (seq-filter (lambda (item)
+                (not (and (eq (nth 1 item) 'custom-variable)
+                          (get (nth 0 item) 'byte-obsolete-variable))))
+              items))
+
 (defun custom-group-members (symbol groups-only)
   "Return SYMBOL's custom group members.
 If GROUPS-ONLY is non-nil, return only those members that are groups."
@@ -4437,12 +4413,13 @@ custom-group-value-create
 					     ?\s))
 	   ;; Members.
 	   (message "Creating group...")
-	   (let* ((members (custom-sort-items
-			    members
-			    ;; Never sort the top-level custom group.
-			    (unless (eq symbol 'emacs)
-			      custom-buffer-sort-alphabetically)
-			    custom-buffer-order-groups))
+           (let* ((members (custom--filter-obsolete-variables
+                            (custom-sort-items
+                             members
+                             ;; Never sort the top-level custom group.
+                             (unless (eq symbol 'emacs)
+                               custom-buffer-sort-alphabetically)
+                             custom-buffer-order-groups)))
 		  (prefixes (widget-get widget :custom-prefixes))
 		  (custom-prefix-list (custom-prefix-add symbol prefixes))
 		  (have-subtitle (and (not (eq symbol 'emacs))
diff --git a/test/lisp/cus-edit-tests.el b/test/lisp/cus-edit-tests.el
new file mode 100644
index 0000000000..6fe1ce4a11
--- /dev/null
+++ b/test/lisp/cus-edit-tests.el
@@ -0,0 +1,46 @@
+;;; cus-edit-tests.el --- Tests for cus-edit.el  -*- lexical-binding: t -*-
+
+;; Copyright (C) 2020 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+(require 'cus-edit)
+
+(defgroup cus-edit-tests nil "test"
+  :group 'test-group)
+
+(defcustom cus-edit--testvar-obsolete nil "test"
+  :type 'boolean)
+(make-obsolete-variable 'cus-edit--testvar-obsolete nil "X.X-test")
+
+(ert-deftest cus-edit-tests-customize-group/no-obsolete ()
+  "Check that obsolete variables do not show up."
+  (save-window-excursion
+    (unwind-protect
+        (progn
+          (customize-group 'cus-edit-tests)
+          (should-not (search-forward "Cus Edit Testvar Obsolete" nil t)))
+      (when-let ((buf (get-buffer "*Customize Group: Cus Edit Tests*")))
+        (kill-buffer buf)))))
+
+(provide 'cus-edit-tests)
+;;; cus-edit-tests.el ends here
-- 
2.28.0


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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-12 20:56   ` bug#44598: [PATCH] Do not show obsolete options in customize Stefan Kangas
@ 2020-11-12 21:11     ` Basil L. Contovounesios
  2020-11-12 21:39       ` Stefan Kangas
  2020-11-13  7:43       ` Eli Zaretskii
  2020-11-12 21:40     ` Drew Adams
                       ` (2 subsequent siblings)
  3 siblings, 2 replies; 14+ messages in thread
From: Basil L. Contovounesios @ 2020-11-12 21:11 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44598

Stefan Kangas <stefan@marxist.se> writes:

> From 66221f1d0f7ee4f2af0d6c65fe956cce711b48e2 Mon Sep 17 00:00:00 2001
> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 24 Oct 2020 19:44:20 +0200
> Subject: [PATCH] Do not show obsolete options in customize
>
> * lisp/cus-edit.el (custom-variable-obsolete): Delete face.

Perhaps it should be marked obsolete first?

Thanks,

-- 
Basil





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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-12 21:11     ` Basil L. Contovounesios
@ 2020-11-12 21:39       ` Stefan Kangas
  2020-11-12 22:18         ` Basil L. Contovounesios
  2020-11-13  7:43       ` Eli Zaretskii
  1 sibling, 1 reply; 14+ messages in thread
From: Stefan Kangas @ 2020-11-12 21:39 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 44598

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> From 66221f1d0f7ee4f2af0d6c65fe956cce711b48e2 Mon Sep 17 00:00:00 2001
>> From: Stefan Kangas <stefan@marxist.se>
>> Date: Sat, 24 Oct 2020 19:44:20 +0200
>> Subject: [PATCH] Do not show obsolete options in customize
>>
>> * lisp/cus-edit.el (custom-variable-obsolete): Delete face.
>
> Perhaps it should be marked obsolete first?

It's not clear to me how this is supposed to work.  I see some patches
in the git log that just deletes them, and some that don't.

We also have `define-obsolete-face-alias' but no `make-obsolete-face'.





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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-12 20:56   ` bug#44598: [PATCH] Do not show obsolete options in customize Stefan Kangas
  2020-11-12 21:11     ` Basil L. Contovounesios
@ 2020-11-12 21:40     ` Drew Adams
  2020-11-12 21:44     ` Mauro Aranda
  2020-11-13  7:40     ` Eli Zaretskii
  3 siblings, 0 replies; 14+ messages in thread
From: Drew Adams @ 2020-11-12 21:40 UTC (permalink / raw)
  To: Stefan Kangas, 44598

> In a recent discussion on emacs-devel,[1]

Wow.  I missed that.  I've responded there now.

> > IMO, we shouldn't show obsolete options at all in a Custom buffer,
> > for the same reason why we remove them from the manuals.
> 
> How about the attached patch?

FTR, I disagree with this approach.  I think it hurts
more than it helps.

If customizing some _particular_ option has a very
negative effect, then it could be dealt with specially.
But to just remove customization of all obsolete options
makes no sense to me.  And "for the same reason" as not
documenting something that's obsolete doesn't sound like
a sound reason to me, at all.  Doc is one thing.  Use is
another.





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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-12 20:56   ` bug#44598: [PATCH] Do not show obsolete options in customize Stefan Kangas
  2020-11-12 21:11     ` Basil L. Contovounesios
  2020-11-12 21:40     ` Drew Adams
@ 2020-11-12 21:44     ` Mauro Aranda
  2020-11-12 22:08       ` Stefan Kangas
  2020-11-13  7:48       ` Eli Zaretskii
  2020-11-13  7:40     ` Eli Zaretskii
  3 siblings, 2 replies; 14+ messages in thread
From: Mauro Aranda @ 2020-11-12 21:44 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44598

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

Stefan Kangas <stefan@marxist.se> writes:

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -706,6 +706,10 @@ This file was a compatibility kludge which is no
longer needed.
>  To revert to the previous behavior,
>  '(setq lisp-indent-function 'lisp-indent-function)' from
'lisp-mode-hook'.
>
> +** Customize
> +
> +*** Customize will no longer show obsolete user options.
> +

Only when customizing a group, it seems.  They still show up when using
customize-apropos (quite common), customize-saved (for an old setting,
it could be somewhat common), or when asking to customize them directly
(although that could be less common).  I don't know what others think,
but perhaps customize-saved should still show them: after all, it is a
current user saved setting.

> --- a/lisp/cus-edit.el
> +++ b/lisp/cus-edit.el
> @@ -2505,18 +2505,6 @@ custom-comment-invisible-p
>
>  ;;; The `custom-variable' Widget.
>
> -(defface custom-variable-obsolete
> -  '((((class color) (background dark))
> -     :foreground "light blue")
> -    (((min-colors 88) (class color) (background light))
> -     :foreground "blue1")
> -    (((class color) (background light))
> -     :foreground "blue")
> -    (t :slant italic))
> -  "Face used for obsolete variables."
> -  :version "27.1"
> -  :group 'custom-faces)
> -

Because of the above, perhaps it's too early to remove it?

> @@ -2544,7 +2532,7 @@ custom-variable-documentation
>  Normally just return the docstring.  But if VARIABLE automatically
>  becomes buffer local when set, append a message to that effect.
>  Also append any obsolescence information."
> -  (format "%s%s%s" (documentation-property variable
'variable-documentation t)
> +  (format "%s%s" (documentation-property variable
'variable-documentation t)
>    (if (and (local-variable-if-set-p variable)
>     (or (not (local-variable-p variable))
>         (with-temp-buffer
> @@ -2552,21 +2540,7 @@ custom-variable-documentation
>        "\n
>  This variable automatically becomes buffer-local when set outside Custom.
>  However, setting it through Custom sets the default value."
> -    "")
> -  ;; This duplicates some code from describe-variable.
> -  ;; TODO extract to separate utility function?
> -  (let* ((obsolete (get variable 'byte-obsolete-variable))
> - (use (car obsolete)))
> -    (if obsolete
> - (concat "\n
> -This variable is obsolete"
> - (if (nth 2 obsolete)
> -    (format " since %s" (nth 2 obsolete)))
> - (cond ((stringp use) (concat ";\n" use))
> -      (use (format-message ";\nuse `%s' instead."
> -   (car obsolete)))
> -      (t ".")))
> -      ""))))
> +            "")))

And the same goes for this: if the option is still likely to pop up in
some other Custom buffer, then this is useful information we might want
to keep showing to the user.

> +(defun custom--filter-obsolete-variables (items)
> +  "Filter obsolete variables from ITEMS."
> +  (seq-filter (lambda (item)
> +                (not (and (eq (nth 1 item) 'custom-variable)
> +                          (get (nth 0 item) 'byte-obsolete-variable))))
> +              items))
> +

Nit: perhaps seq-remove?

[-- Attachment #2: Type: text/html, Size: 3986 bytes --]

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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-12 21:44     ` Mauro Aranda
@ 2020-11-12 22:08       ` Stefan Kangas
  2020-11-12 22:41         ` Mauro Aranda
  2020-11-13  7:48       ` Eli Zaretskii
  1 sibling, 1 reply; 14+ messages in thread
From: Stefan Kangas @ 2020-11-12 22:08 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: 44598

Mauro Aranda <maurooaranda@gmail.com> writes:

>> +*** Customize will no longer show obsolete user options.
>> +
>
> Only when customizing a group, it seems.  They still show up when using
> customize-apropos (quite common), customize-saved (for an old setting,
> it could be somewhat common), or when asking to customize them directly
> (although that could be less common).  I don't know what others think,
> but perhaps customize-saved should still show them: after all, it is a
> current user saved setting.

Thanks, I overlooked that.

I think `customize-option' and `customize-saved' should still show
them, indeed.

Excluding the above, is `customize-apropos' otherwise an exhaustive list
of the commands where they would still be visible?

I never use `customize-apropos', so what do you think makes sense for
that command?  Should it still show it?

> Because of the above, perhaps it's too early to remove it?

Perhaps, yes.  I could just mention in its docstring that it's obsolete
instead, since I can't find any facilities to mark a defface obsolete.
Or maybe someone will enlighten me and tell me how it's done...

> And the same goes for this: if the option is still likely to pop up in
> some other Custom buffer, then this is useful information we might want
> to keep showing to the user.

Good point.  I'll take a look at what happens with `customize-option' in
particular, where we would want to mention that information.

> Nit: perhaps seq-remove?

Sure.





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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-12 21:39       ` Stefan Kangas
@ 2020-11-12 22:18         ` Basil L. Contovounesios
  0 siblings, 0 replies; 14+ messages in thread
From: Basil L. Contovounesios @ 2020-11-12 22:18 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44598

Stefan Kangas <stefan@marxist.se> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> Stefan Kangas <stefan@marxist.se> writes:
>>
>>> From 66221f1d0f7ee4f2af0d6c65fe956cce711b48e2 Mon Sep 17 00:00:00 2001
>>> From: Stefan Kangas <stefan@marxist.se>
>>> Date: Sat, 24 Oct 2020 19:44:20 +0200
>>> Subject: [PATCH] Do not show obsolete options in customize
>>>
>>> * lisp/cus-edit.el (custom-variable-obsolete): Delete face.
>>
>> Perhaps it should be marked obsolete first?
>
> It's not clear to me how this is supposed to work.  I see some patches
> in the git log that just deletes them, and some that don't.
>
> We also have `define-obsolete-face-alias' but no `make-obsolete-face'.

Looks like face obsoletion is not as first-class a citizen as variable
and function obsoletion.  If you'd rather not bother, that's fine with
me.

-- 
Basil





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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-12 22:08       ` Stefan Kangas
@ 2020-11-12 22:41         ` Mauro Aranda
  0 siblings, 0 replies; 14+ messages in thread
From: Mauro Aranda @ 2020-11-12 22:41 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44598

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

Stefan Kangas <stefan@marxist.se> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>>> +*** Customize will no longer show obsolete user options.
>>> +
>>
>> Only when customizing a group, it seems.  They still show up when using
>> customize-apropos (quite common), customize-saved (for an old setting,
>> it could be somewhat common), or when asking to customize them directly
>> (although that could be less common).  I don't know what others think,
>> but perhaps customize-saved should still show them: after all, it is a
>> current user saved setting.
>
> Thanks, I overlooked that.
>
> I think `customize-option' and `customize-saved' should still show
> them, indeed.
>
> Excluding the above, is `customize-apropos' otherwise an exhaustive list
> of the commands where they would still be visible?

There's customize-changed-options too.

> I never use `customize-apropos', so what do you think makes sense for
> that command?  Should it still show it?

To me, it makes sense that it follows what customize-group does, in this
regard.

>> Because of the above, perhaps it's too early to remove it?
>
> Perhaps, yes.  I could just mention in its docstring that it's obsolete
> instead, since I can't find any facilities to mark a defface obsolete.
> Or maybe someone will enlighten me and tell me how it's done...

But if one Custom buffer still shows obsolete options, wouldn't the face
still be in use?

>> And the same goes for this: if the option is still likely to pop up in
>> some other Custom buffer, then this is useful information we might want
>> to keep showing to the user.
>
> Good point.  I'll take a look at what happens with `customize-option' in
> particular, where we would want to mention that information.

Why not in the current place?

[-- Attachment #2: Type: text/html, Size: 2254 bytes --]

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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-12 20:56   ` bug#44598: [PATCH] Do not show obsolete options in customize Stefan Kangas
                       ` (2 preceding siblings ...)
  2020-11-12 21:44     ` Mauro Aranda
@ 2020-11-13  7:40     ` Eli Zaretskii
  2020-11-13 17:10       ` Stefan Kangas
  3 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2020-11-13  7:40 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44598

> Cc: eliz@gnu.org
> From: Stefan Kangas <stefan@marxist.se>
> Date: Thu, 12 Nov 2020 15:56:24 -0500
> 
> In a recent discussion on emacs-devel,[1]

You mean this one:

  https://lists.gnu.org/archive/html/emacs-devel/2020-09/msg01617.html

> > IMO, we shouldn't show obsolete options at all in a Custom buffer, for
> > the same reason why we remove them from the manuals.
> 
> How about the attached patch?

Thanks, some comments:

> +** Customize
> +
> +*** Customize will no longer show obsolete user options.

This is too general, it sounds like those options will not be shown
even if the user types "M-x customize-variable" to explicitly
customize an obsolete option.  That's not what the patch does, does
it?

> -(defface custom-variable-obsolete
> -  '((((class color) (background dark))
> -     :foreground "light blue")
> -    (((min-colors 88) (class color) (background light))
> -     :foreground "blue1")
> -    (((class color) (background light))
> -     :foreground "blue")
> -    (t :slant italic))
> -  "Face used for obsolete variables."
> -  :version "27.1"
> -  :group 'custom-faces)

You are removing the face, but then how will we display obsolete
options when the user actually asks to customize them by name?

> +(defun custom--filter-obsolete-variables (items)
> +  "Filter obsolete variables from ITEMS."

This doesn't say what kind of object is ITEMS expected to be.

> +  (seq-filter (lambda (item)
> +                (not (and (eq (nth 1 item) 'custom-variable)
> +                          (get (nth 0 item) 'byte-obsolete-variable))))
> +              items))

Do we really need the power of seq.el here? wouldn't mapcar do the job
nicely, since we have a simple list here?  OTOH, if you do use seq.el,
why seq-filter and not seq-remove?

> +(ert-deftest cus-edit-tests-customize-group/no-obsolete ()
> +  "Check that obsolete variables do not show up."
> +  (save-window-excursion
> +    (unwind-protect
> +        (progn
> +          (customize-group 'cus-edit-tests)
> +          (should-not (search-forward "Cus Edit Testvar Obsolete" nil t)))
> +      (when-let ((buf (get-buffer "*Customize Group: Cus Edit Tests*")))
> +        (kill-buffer buf)))))

This test will fail when we remove the obsolete option, right?  How
can we come up with a test that doesn't suffer from such maintenance
problems?





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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-12 21:11     ` Basil L. Contovounesios
  2020-11-12 21:39       ` Stefan Kangas
@ 2020-11-13  7:43       ` Eli Zaretskii
  1 sibling, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2020-11-13  7:43 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: stefan, 44598

> From: "Basil L. Contovounesios" <contovob@tcd.ie>
> Date: Thu, 12 Nov 2020 21:11:07 +0000
> Cc: 44598@debbugs.gnu.org
> 
> Stefan Kangas <stefan@marxist.se> writes:
> 
> > From 66221f1d0f7ee4f2af0d6c65fe956cce711b48e2 Mon Sep 17 00:00:00 2001
> > From: Stefan Kangas <stefan@marxist.se>
> > Date: Sat, 24 Oct 2020 19:44:20 +0200
> > Subject: [PATCH] Do not show obsolete options in customize
> >
> > * lisp/cus-edit.el (custom-variable-obsolete): Delete face.
> 
> Perhaps it should be marked obsolete first?

You mean, literally an obsolete face?  Or did you mean a new variable
that would allow displaying the obsolete option as we do today?  For
the latter, it might make sense, as a "fire escape" for people who
want the old behavior.  For the former, perhaps we shouldn't remove
the face at all, see my other mail.





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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-12 21:44     ` Mauro Aranda
  2020-11-12 22:08       ` Stefan Kangas
@ 2020-11-13  7:48       ` Eli Zaretskii
  1 sibling, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2020-11-13  7:48 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: stefan, 44598

> From: Mauro Aranda <maurooaranda@gmail.com>
> Date: Thu, 12 Nov 2020 18:44:24 -0300
> Cc: 44598@debbugs.gnu.org
> 
> I don't know what others think, but perhaps customize-saved should
> still show them: after all, it is a current user saved setting.

Yes, I think I agree.

> > -(defface custom-variable-obsolete
> > -  '((((class color) (background dark))
> > -     :foreground "light blue")
> > -    (((min-colors 88) (class color) (background light))
> > -     :foreground "blue1")
> > -    (((class color) (background light))
> > -     :foreground "blue")
> > -    (t :slant italic))
> > -  "Face used for obsolete variables."
> > -  :version "27.1"
> > -  :group 'custom-faces)
> > -
> 
> Because of the above, perhaps it's too early to remove it?

Yes.





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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-13  7:40     ` Eli Zaretskii
@ 2020-11-13 17:10       ` Stefan Kangas
  2020-11-14 14:22         ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Kangas @ 2020-11-13 17:10 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 44598

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

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, some comments:

Thanks for commenting, as always.

Please find attached an updated patch that should fix all your comments.

> Do we really need the power of seq.el here? wouldn't mapcar do the job
> nicely, since we have a simple list here?  OTOH, if you do use seq.el,
> why seq-filter and not seq-remove?

I think mapcar won't work since the returned list is the same length as
its input.  seq-remove seems better indeed.

> This test will fail when we remove the obsolete option, right?  How
> can we come up with a test that doesn't suffer from such maintenance
> problems?

This option is only defined in this file for testing purposes, and
should never be removed.  I've clarified this in its docstring and added
a high version number.

[-- Attachment #2: 0001-Hide-obsolete-options-in-most-customize-commands.patch --]
[-- Type: text/x-diff, Size: 7067 bytes --]

From 58b144a805dd46b26db4fa266ac124af3146448e Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Sat, 24 Oct 2020 19:44:20 +0200
Subject: [PATCH] Hide obsolete options in most customize commands

* lisp/cus-edit.el (custom--filter-obsolete-variables): New defun.
* lisp/cus-edit.el (customize-changed-options)
(customize-apropos, custom-group-value-create): Hide obsolete user
options.  (Bug#44598)
* test/lisp/cus-edit-tests.el: New file.
---
 etc/NEWS                    | 10 +++++
 lisp/cus-edit.el            | 26 ++++++++----
 test/lisp/cus-edit-tests.el | 80 +++++++++++++++++++++++++++++++++++++
 3 files changed, 108 insertions(+), 8 deletions(-)
 create mode 100644 test/lisp/cus-edit-tests.el

diff --git a/etc/NEWS b/etc/NEWS
index f21c4cb02c..fa9a87d80c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -706,6 +706,16 @@ This file was a compatibility kludge which is no longer needed.
 To revert to the previous behavior,
 '(setq lisp-indent-function 'lisp-indent-function)' from 'lisp-mode-hook'.
 
+** Customize
+
+*** Most customize commands now hides obsolete user options.
+Obsolete user options are no longer shown in the listings produced by
+the commands `customize', `customize-group', `customize-apropos' and
+`customize-changed-options'.
+
+To customize obsolete user options, use `customize-option' or
+`customize-saved'.
+
 ** Edebug
 
 +++
diff --git a/lisp/cus-edit.el b/lisp/cus-edit.el
index d1077d367d..b46be39e38 100644
--- a/lisp/cus-edit.el
+++ b/lisp/cus-edit.el
@@ -1298,7 +1298,8 @@ customize-changed-options
 	       (if (custom-facep symbol)
 		   (push (list symbol 'custom-face) found)))))))
     (if found
-	(custom-buffer-create (custom-sort-items found t 'first)
+        (custom-buffer-create (custom--filter-obsolete-variables
+                               (custom-sort-items found t 'first))
 			      "*Customize Changed Options*")
       (user-error "No user option defaults have been changed since Emacs %s"
                   since-version))))
@@ -1504,7 +1505,8 @@ customize-apropos
 						(symbol-name type))
 	     pattern))
     (custom-buffer-create
-     (custom-sort-items found t custom-buffer-order-groups)
+     (custom--filter-obsolete-variables
+      (custom-sort-items found t custom-buffer-order-groups))
      "*Customize Apropos*")))
 
 ;;;###autoload
@@ -4232,6 +4234,13 @@ custom-group-visibility-create
 	(insert "--------")))
   (widget-default-create widget))
 
+(defun custom--filter-obsolete-variables (items)
+  "Filter obsolete variables from ITEMS."
+  (seq-remove (lambda (item)
+                (and (eq (nth 1 item) 'custom-variable)
+                     (get (nth 0 item) 'byte-obsolete-variable)))
+              items))
+
 (defun custom-group-members (symbol groups-only)
   "Return SYMBOL's custom group members.
 If GROUPS-ONLY is non-nil, return only those members that are groups."
@@ -4437,12 +4446,13 @@ custom-group-value-create
 					     ?\s))
 	   ;; Members.
 	   (message "Creating group...")
-	   (let* ((members (custom-sort-items
-			    members
-			    ;; Never sort the top-level custom group.
-			    (unless (eq symbol 'emacs)
-			      custom-buffer-sort-alphabetically)
-			    custom-buffer-order-groups))
+           (let* ((members (custom--filter-obsolete-variables
+                            (custom-sort-items
+                             members
+                             ;; Never sort the top-level custom group.
+                             (unless (eq symbol 'emacs)
+                               custom-buffer-sort-alphabetically)
+                             custom-buffer-order-groups)))
 		  (prefixes (widget-get widget :custom-prefixes))
 		  (custom-prefix-list (custom-prefix-add symbol prefixes))
 		  (have-subtitle (and (not (eq symbol 'emacs))
diff --git a/test/lisp/cus-edit-tests.el b/test/lisp/cus-edit-tests.el
new file mode 100644
index 0000000000..4d148b4f41
--- /dev/null
+++ b/test/lisp/cus-edit-tests.el
@@ -0,0 +1,80 @@
+;;; cus-edit-tests.el --- Tests for cus-edit.el  -*- lexical-binding: t -*-
+
+;; Copyright (C) 2020 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'ert)
+(require 'ert-x)
+(require 'cus-edit)
+
+(defmacro with-cus-edit-test (buffer &rest body)
+  (declare (indent 1))
+  `(save-window-excursion
+     (unwind-protect
+         (progn ,@body)
+       (when-let ((buf (get-buffer ,buffer)))
+         (kill-buffer buf)))))
+
+\f
+;;;; showing/hiding obsolete options
+
+(defgroup cus-edit-tests nil "test"
+  :group 'test-group)
+
+(defcustom cus-edit-tests--obsolete-option-tag nil
+  "This should never be removed; it is obsolete for testing purposes."
+  :type 'boolean
+  :version "917.10") ; a super high version number
+(make-obsolete-variable 'cus-edit-tests--obsolete-option-tag nil "X.X-test")
+(defconst cus-edit-tests--obsolete-option-tag
+  (custom-unlispify-tag-name 'cus-edit-tests--obsolete-option-tag))
+
+(ert-deftest cus-edit-tests-customize-apropos/hide-obsolete ()
+  (with-cus-edit-test "*Customize Apropos*"
+    (customize-apropos "cus-edit-tests")
+    (should-not (search-forward cus-edit-tests--obsolete-option-tag nil t))))
+
+(ert-deftest cus-edit-tests-customize-changed-options/hide-obsolete ()
+  (with-cus-edit-test "*Customize Changed Options*"
+    (customize-changed-options "917.2") ; some future version
+    (should-not (search-forward cus-edit-tests--obsolete-option-tag nil t))))
+
+(ert-deftest cus-edit-tests-customize-group/hide-obsolete ()
+  "Check that obsolete variables do not show up."
+  (with-cus-edit-test "*Customize Group: Cus Edit Tests*"
+    (customize-group 'cus-edit-tests)
+    (should-not (search-forward cus-edit-tests--obsolete-option-tag nil t))))
+
+(ert-deftest cus-edit-tests-customize-option/show-obsolete ()
+  (with-cus-edit-test "*Customize Option: Cus Edit Tests Obsolete Option Tag*"
+    (customize-option 'cus-edit-tests--obsolete-option-tag)
+    (goto-char (point-min))
+    (should (search-forward cus-edit-tests--obsolete-option-tag nil t))))
+
+(ert-deftest cus-edit-tests-customize-saved/show-obsolete ()
+  ;; FIXME: How to test for saved options?
+  :expected-result :failed
+  (with-cus-edit-test "*Customize Saved*"
+    (customize-saved)
+    (should (search-forward cus-edit-tests--obsolete-option-tag nil t))))
+
+(provide 'cus-edit-tests)
+;;; cus-edit-tests.el ends here
-- 
2.29.2


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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-13 17:10       ` Stefan Kangas
@ 2020-11-14 14:22         ` Eli Zaretskii
  2020-11-20 13:37           ` Stefan Kangas
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2020-11-14 14:22 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 44598

> From: Stefan Kangas <stefan@marxist.se>
> Date: Fri, 13 Nov 2020 09:10:52 -0800
> Cc: 44598@debbugs.gnu.org
> 
> +** Customize
> +
> +*** Most customize commands now hides obsolete user options.
                                   ^^^^^
"hide", plural.

Otherwise, LGTM, thanks.





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

* bug#44598: [PATCH] Do not show obsolete options in customize
  2020-11-14 14:22         ` Eli Zaretskii
@ 2020-11-20 13:37           ` Stefan Kangas
  0 siblings, 0 replies; 14+ messages in thread
From: Stefan Kangas @ 2020-11-20 13:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 44598

tags 44598 fixed
close 44598 28.1
thanks

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Kangas <stefan@marxist.se>
>> Date: Fri, 13 Nov 2020 09:10:52 -0800
>> Cc: 44598@debbugs.gnu.org
>>
>> +** Customize
>> +
>> +*** Most customize commands now hides obsolete user options.
>                                    ^^^^^
> "hide", plural.

Fixed.

> Otherwise, LGTM, thanks.

Thanks, pushed to master as commit b4b1bd6e03.





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

end of thread, other threads:[~2020-11-20 13:37 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CADwFkmm2G=OPOdgadhDk+1uCbHzuqpqaYDs1KgdDes7gXLYgxg@mail.gmail.com>
     [not found] ` <83lfh743j8.fsf@gnu.org>
2020-11-12 20:56   ` bug#44598: [PATCH] Do not show obsolete options in customize Stefan Kangas
2020-11-12 21:11     ` Basil L. Contovounesios
2020-11-12 21:39       ` Stefan Kangas
2020-11-12 22:18         ` Basil L. Contovounesios
2020-11-13  7:43       ` Eli Zaretskii
2020-11-12 21:40     ` Drew Adams
2020-11-12 21:44     ` Mauro Aranda
2020-11-12 22:08       ` Stefan Kangas
2020-11-12 22:41         ` Mauro Aranda
2020-11-13  7:48       ` Eli Zaretskii
2020-11-13  7:40     ` Eli Zaretskii
2020-11-13 17:10       ` Stefan Kangas
2020-11-14 14:22         ` Eli Zaretskii
2020-11-20 13:37           ` Stefan Kangas

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).