bug#38675: 27.0.50; define-obsolete-face-alias definition does not match documentation

bug#38675: 27.0.50; define-obsolete-face-alias definition does not match documentation

[James Felix Black]

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

This was discovered in `haskell-mode` [1]. The documentation of  define-obsolete-face-alias claims that `when` is optional, but the macro definition does not follow the docs. This patch fixes the problem.

In GNU Emacs 27.0.50 (build 1, x86_64-apple-darwin19.2.0, NS appkit-1894.20 Version 10.15.2 (Build 19C57))
 of 2019-12-19 built on moomin.local
Repository revision: f38e8a86650e8a7f72293d81a7392289c298ae5a
Repository branch: fix_define-obsolete-face-alias_optional
Windowing system distributor 'Apple', version 10.3.1894
System Description:  Mac OS X 10.15.2

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
user-error: The mark is not set now, so there is no region
Making completion list...

Configured features:
NOTIFY KQUEUE ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS MODULES
THREADS PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_CA.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml easymenu mml-sec password-cache epa derived epg
epg-config gnus-util rmail rmail-loaddefs text-property-search time-date
subr-x seq byte-opt gv bytecomp byte-compile cconv mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader cl-loaddefs
cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/ns-win ns-win ucs-normalize mule-util term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
tab-bar menu-bar rfn-eshadow isearch timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core term/tty-colors frame
minibuffer cl-generic cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite charscript charprop case-table epa-hook jka-cmpr-hook help
simple abbrev obarray cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
threads kqueue cocoa ns lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 43939 6393)
 (symbols 48 5909 1)
 (strings 32 15245 1247)
 (string-bytes 1 502913)
 (vectors 16 9297)
 (vector-slots 8 118962 11572)
 (floats 8 20 25)
 (intervals 56 185 0)
 (buffers 1000 12))



[1] https://github.com/haskell/haskell-mode/issues/1691


[-- Attachment #2: 0001-Fix-macro-definition.patch --]
[-- Type: application/octet-stream, Size: 1282 bytes --]

From f38e8a86650e8a7f72293d81a7392289c298ae5a Mon Sep 17 00:00:00 2001
From: James Felix Black <jfb@homonculus.net>
Date: Thu, 19 Dec 2019 12:29:11 -0500
Subject: [PATCH] Fix macro definition

`define-obsolete-face-alias` claims not to require the third argument,
`when`. However, `when` is not defined `&optional`. This patch addresses
this.
---
 lisp/emacs-lisp/byte-run.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el
index 1115c09667..27aee3ed4e 100644
--- a/lisp/emacs-lisp/byte-run.el
+++ b/lisp/emacs-lisp/byte-run.el
@@ -448,7 +448,7 @@ CURRENT-NAME, if it does not already have them:
 ;; by the byte-compiler (would be nice if it could warn about obsolete
 ;; faces, but it doesn't really do anything special with faces).
 ;; It only really affects M-x describe-face output.
-(defmacro define-obsolete-face-alias (obsolete-face current-face when)
+(defmacro define-obsolete-face-alias (obsolete-face current-face &optional when)
   "Make OBSOLETE-FACE a face alias for CURRENT-FACE and mark it obsolete.
 If provided, WHEN should be a string indicating when the face
 was first made obsolete, for example a date or a release number."
-- 
2.21.0 (Apple Git-122.2)

bug#38675: 27.0.50; define-obsolete-face-alias definition does not match documentation

[Eli Zaretskii]

> From: James Felix Black <jfb@homonculus.net>
> Date: Thu, 19 Dec 2019 12:49:01 -0500
> 
> This was discovered in `haskell-mode` [1]. The documentation of  define-obsolete-face-alias claims that `when` is optional, but the macro definition does not follow the docs. This patch fixes the problem.

I'm not sure this isn't a confusion caused by ambiguous wording of the
doc string.  I think by "if provided" it means that WHEN can be nil.
The ELisp manual, btw, doesn't have this problem.

Would it be okay to modify the doc string instead to say WHEN is
required, but can be nil?

Thanks.

bug#38675: 27.0.50; define-obsolete-face-alias definition does not match documentation

[Glenn Morris]

ced10a4c9f changed the arg spec but not the doc (or code).
The intent seems clear: a proper "when" should always be provided.

bug#38675: 27.0.50; define-obsolete-face-alias definition does not match documentation

[James Felix Black]

I’m easy either way. Do the usage and docs match the other “define-obsolete-“ macros?

> On Dec 21, 2019, at 04:38, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> 
>> 
>> From: James Felix Black <jfb@homonculus.net>
>> Date: Thu, 19 Dec 2019 12:49:01 -0500
>> 
>> This was discovered in `haskell-mode` [1]. The documentation of  define-obsolete-face-alias claims that `when` is optional, but the macro definition does not follow the docs. This patch fixes the problem.
> 
> I'm not sure this isn't a confusion caused by ambiguous wording of the
> doc string.  I think by "if provided" it means that WHEN can be nil.
> The ELisp manual, btw, doesn't have this problem.
> 
> Would it be okay to modify the doc string instead to say WHEN is
> required, but can be nil?
> 
> Thanks.

bug#38675: 27.0.50; define-obsolete-face-alias definition does not match documentation

[Eli Zaretskii]

> From: James Felix Black <jfb@homonculus.net>
> Date: Sat, 21 Dec 2019 17:47:45 -0500
> Cc: 38675@debbugs.gnu.org
> 
> I’m easy either way. Do the usage and docs match the other “define-obsolete-“ macros?

I fixed the documentation to fit the advertised calling convention,
and I'm therefore closing this bug report.

Thanks.