bug#48404: 28.0.50; "Invalid version syntax" errors with read-extended-command

[Eli Zaretskii]

> From: Tassilo Horn <tsdh@gnu.org>
> Cc: 48404@debbugs.gnu.org, stefan@marxist.se
> Date: Fri, 14 May 2021 08:47:32 +0200
> 
> > In any case, it sounds like blindly trusting the value of that
> > property is not a good idea, and we should wrap the call to
> > version-to-list there in condition-case.  Could you try that?
> 
> I've tried this
> 
> --8<---------------cut here---------------start------------->8---
> diff --git a/lisp/simple.el b/lisp/simple.el
> index 0255f69e42..573e344fea 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -2020,8 +2020,10 @@ read-extended-command
>                                      ;; Has a current-name.
>                                      (functionp (car obsolete))
>                                      ;; when >= emacs-major-version
> -                                    (>= (car (version-to-list (caddr 
> obsolete)))
> -                                        emacs-major-version))))))
> +                                    (condition-case nil
> +                                        (>= (car (version-to-list (caddr 
> obsolete)))
> +                                            emacs-major-version)
> +                                      (t nil)))))))
>                      pred)))
>               (complete-with-action action obarray string pred))))
> --8<---------------cut here---------------end--------------->8---
> 
> which does the trick.

Thanks, I think this should be installed unless someone comes up with
a better idea (but see below).

> Should I write bug reports for Magit and Yasnippets so that they should
> only specify the version number in `make-obsolete' /
> `make-obsolete-variable'?  Or should emacs support the "<package>
> <version>" syntax?

I don't think we will support the deviant syntax in version-to-list,
no.

But I don't really understand why we compare versions in the snippet
above -- the command is already established as being obsolete, so
we've got to be running an Emacs version where it is indeed obsolete,
and the version text sounds redundant to me.  Stefan?

If we remove the version check, the problem with 3rd-party packages
will be automatically resolved.  Otherwise, I don't think we can solve
this except by ignoring those non-Emacs versions, because there's no
way for us to know whether, e.g., "Magit 3.0.0" is older or newer that
Emacs whose version is emacs-major-version.  IOW, if we insist on
checking the version, AFAIU this feature cannot possibly work for
obsolete commands from 3rd-party packages, they will always be
included in completion lists.