Since commit 1af91d271e077134e272055407fb8c4312a7579b I get strange "Invalid version syntax: <package> <version> (must start with a number)" errors during M-x completion in the minibuffer. I can't reproduce with emacs -Q but in the error, <package> is always some (M)ELPA package. For example, M-x mag TAB gives completion--some: Invalid version syntax: ‘Magit 3.0.0’ (must start with a number) Here is a backtrace: --8<---------------cut here---------------start------------->8--- Debugger entered--Lisp error: (error "Invalid version syntax: ‘Magit 3.0.0’ (must start ...") error("Invalid version syntax: `%s' (must start with a nu..." "Magit 3.0.0") version-to-list("Magit 3.0.0") #f(compiled-function (sym) #<bytecode -0x190dc04a7f35bb9f>)(magit-dispatch-popup) try-completion("mag" [## magit-section-show-headings vc-src-responsible-p Read\ manual tag-end dashboard-ls Set\ Slice debbugs-gnu-log-edit-mode mu4e-kill-update-mail Reduce\ To:\ to\ Cc: mu4e-view-header-field-keymap c-electric-lt-gt :neither yas-x-prompt cc-vars rainbow-colorize-ansi znc magit-xref-insert-button inactive gnus-agent-group-covered-p web-mode-extra-snippets smiley-directory message-expand-name-databases transient:magit-diff-refresh:--irreversible-delete er/looking-back-max tree-widget-button-keymap rust--same-line-p gnus-summary-show-article-from-menu-as-charset-ebcdic-int :where org-switch-to-buffer-other-window nexus th/visual-line-mode-init :mac xref--push-markers vc-bzr-shelve-menu display-even-when-displayed widget-face-sample-face-get time-subsecond gnus-summary-limit-to-marks he-line-search-regexp symbol-before-point-for-complete magit-filename utf7-direct-encoding-chars generic--normalize-comments org-get-org-file check-declare xref-group :org-comment stack-mode Boring\ headers ...] #f(compiled-function (sym) #<bytecode -0x190dc04a7f35bb9f>)) complete-with-action(nil [## magit-section-show-headings vc-src-responsible-p Read\ manual tag-end dashboard-ls Set\ Slice debbugs-gnu-log-edit-mode mu4e-kill-update-mail Reduce\ To:\ to\ Cc: mu4e-view-header-field-keymap c-electric-lt-gt :neither yas-x-prompt cc-vars rainbow-colorize-ansi znc magit-xref-insert-button inactive gnus-agent-group-covered-p web-mode-extra-snippets smiley-directory message-expand-name-databases transient:magit-diff-refresh:--irreversible-delete er/looking-back-max tree-widget-button-keymap rust--same-line-p gnus-summary-show-article-from-menu-as-charset-ebcdic-int :where org-switch-to-buffer-other-window nexus th/visual-line-mode-init :mac xref--push-markers vc-bzr-shelve-menu display-even-when-displayed widget-face-sample-face-get time-subsecond gnus-summary-limit-to-marks he-line-search-regexp symbol-before-point-for-complete magit-filename utf7-direct-encoding-chars generic--normalize-comments org-get-org-file check-declare xref-group :org-comment stack-mode Boring\ headers ...] "mag" #f(compiled-function (sym) #<bytecode -0x190dc04a7f35bb9f>)) #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_46>("mag" #f(compiled-function (sym) #<bytecode 0x4d1b943a9e2c352>) nil) completion-basic-try-completion("mag" #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_46> #f(compiled-function (sym) #<bytecode 0x4d1b943a9e2c352>) 3) #f(compiled-function (style) #<bytecode 0xaee4d3d0651ffdc>)(basic) completion--some(#f(compiled-function (style) #<bytecode 0xaee4d3d0651ffdc>) (basic partial-completion substring initials flex)) completion--nth-completion(1 "mag" #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_46> #f(compiled-function (sym) #<bytecode 0x4d1b943a9e2c352>) 3 (metadata (affixation-function . read-extended-command--affixation) (category . command))) completion-try-completion("mag" #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_46> #f(compiled-function (sym) #<bytecode 0x4d1b943a9e2c352>) 3 (metadata (affixation-function . read-extended-command--affixation) (category . command))) completion--do-completion(5 8) completion--in-region-1(5 8) #f(compiled-function (start end collection predicate) #<bytecode 0x50e15298b25ed54>)(5 8 #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_46> #f(compiled-function (sym) #<bytecode 0x4d1b943a9e2c352>)) apply(#f(compiled-function (start end collection predicate) #<bytecode 0x50e15298b25ed54>) (5 8 #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_46> #f(compiled-function (sym) #<bytecode 0x4d1b943a9e2c352>))) #f(compiled-function (funs global args) #<bytecode -0xf7a9686f11d622>)(nil nil (5 8 #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_46> #f(compiled-function (sym) #<bytecode 0x4d1b943a9e2c352>))) completion--in-region(5 8 #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_46> #f(compiled-function (sym) #<bytecode 0x4d1b943a9e2c352>)) completion-in-region(5 8 #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_46> #f(compiled-function (sym) #<bytecode 0x4d1b943a9e2c352>)) minibuffer-complete() funcall-interactively(minibuffer-complete) command-execute(minibuffer-complete) completing-read-default("M-x " #<subr F616e6f6e796d6f75732d6c616d626461_anonymous_lambda_46> #f(compiled-function (sym) #<bytecode 0x4d1b943a9e2c352>) t nil extended-command-history nil nil) read-extended-command() byte-code("\302\30\11\303 \10E)\207" [execute-extended-command--last-typed current-prefix-arg nil read-extended-command] 3) command-execute(execute-extended-command) --8<---------------cut here---------------end--------------->8--- In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.29, cairo version 1.17.4) of 2021-05-13 built on thinkpad-t440p Repository revision: 43701a84367b76ccc93ad46f89110486988eec10 Repository branch: master Windowing system distributor 'The X.Org Foundation', version 11.0.12101001 System Description: Arch Linux Configured using: 'configure --with-modules --with-native-compilation' Configured features: ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG JSON LCMS2 LIBOTF LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB Important settings: value of $LC_MONETARY: de_DE.utf8 value of $LC_NUMERIC: de_DE.utf8 value of $LC_TIME: de_DE.utf8 value of $LANG: en_US.UTF-8 locale-coding-system: utf-8-unix Major mode: mu4e:main Minor modes in effect: global-aggressive-indent-mode: t dynamic-completion-mode: t which-key-mode: t company-posframe-mode: t global-company-mode: t yas-global-mode: t yas-minor-mode: t global-git-commit-mode: t magit-auto-revert-mode: t override-global-mode: t minibuffer-depth-indicate-mode: t recentf-mode: t pixel-scroll-mode: t save-place-mode: t savehist-mode: t show-paren-mode: t shell-dirtrack-mode: t tooltip-mode: t global-eldoc-mode: t electric-indent-mode: t mouse-wheel-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 buffer-read-only: t column-number-mode: t line-number-mode: t transient-mark-mode: t overwrite-mode: overwrite-mode-binary Load-path shadows: ~/Repos/el/mu/build/mu4e/mu4e hides ~/Repos/el/mu/mu4e/mu4e ~/Repos/el/mu/build/mu4e/mu4e-main hides ~/Repos/el/mu/mu4e/mu4e-main ~/Repos/el/mu/build/mu4e/mu4e-view hides ~/Repos/el/mu/mu4e/mu4e-view ~/Repos/el/mu/build/mu4e/mu4e-org hides ~/Repos/el/mu/mu4e/mu4e-org ~/Repos/el/mu/build/mu4e/mu4e-lists hides ~/Repos/el/mu/mu4e/mu4e-lists ~/Repos/el/mu/build/mu4e/mu4e-actions hides ~/Repos/el/mu/mu4e/mu4e-actions ~/Repos/el/mu/build/mu4e/mu4e-utils hides ~/Repos/el/mu/mu4e/mu4e-utils ~/Repos/el/mu/build/mu4e/mu4e-context hides ~/Repos/el/mu/mu4e/mu4e-context ~/Repos/el/mu/build/mu4e/mu4e-draft hides ~/Repos/el/mu/mu4e/mu4e-draft ~/Repos/el/mu/build/mu4e/mu4e-message hides ~/Repos/el/mu/mu4e/mu4e-message ~/Repos/el/mu/build/mu4e/mu4e-compose hides ~/Repos/el/mu/mu4e/mu4e-compose ~/Repos/el/mu/build/mu4e/mu4e-view-common hides ~/Repos/el/mu/mu4e/mu4e-view-common ~/Repos/el/mu/build/mu4e/mu4e-view-old hides ~/Repos/el/mu/mu4e/mu4e-view-old ~/Repos/el/mu/build/mu4e/mu4e-view-gnus hides ~/Repos/el/mu/mu4e/mu4e-view-gnus ~/Repos/el/mu/build/mu4e/mu4e-headers hides ~/Repos/el/mu/mu4e/mu4e-headers ~/Repos/el/mu/build/mu4e/mu4e-mark hides ~/Repos/el/mu/mu4e/mu4e-mark ~/Repos/el/mu/build/mu4e/mu4e-icalendar hides ~/Repos/el/mu/mu4e/mu4e-icalendar ~/Repos/el/mu/build/mu4e/mu4e-speedbar hides ~/Repos/el/mu/mu4e/mu4e-speedbar ~/Repos/el/mu/build/mu4e/mu4e-contrib hides ~/Repos/el/mu/mu4e/mu4e-contrib ~/Repos/el/mu/build/mu4e/mu4e-proc hides ~/Repos/el/mu/mu4e/mu4e-proc ~/Repos/el/mu/build/mu4e/mu4e-meta hides ~/Repos/el/mu/mu4e/mu4e-meta ~/Repos/el/mu/build/mu4e/mu4e-vars hides ~/Repos/el/mu/mu4e/mu4e-vars /home/horn/.emacs.d/elpa/transient-20210427.833/transient hides /home/horn/Repos/el/emacs/lisp/transient Features: (shadow hippie-exp emacsbug debbugs-gnu qp sort gnus-cite mail-extr goto-addr magit-extras cursor-sensor face-remap paredit vc-mtn vc-hg vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs display-fill-column-indicator company-oddmuse company-keywords company-etags etags fileloop generator xref project company-gtags company-dabbrev-code company-dabbrev company-files company-clang company-capf company-cmake company-semantic company-template company-bbdb cus-edit pp cus-start cus-load auto-package-update generic yaml-mode fish-mode cargo cargo-process rust-utils rust-mode rust-rustfmt rust-playpen rust-compile rust-cargo web-mode disp-table preview-latex auto-loads tex-site deadgrep spinner hl-todo aggressive-indent rainbow-mode vc-git vc-dir ewoc vc vc-dispatcher epa-file dired-x mu4e-alert time ht s mu4e-icalendar gnus-icalendar org-capture org-refile icalendar diary-lib diary-loaddefs mu4e mu4e-org mu4e-main mu4e-view mu4e-view-gnus mu4e-view-common mu4e-headers mu4e-compose mu4e-context mu4e-draft mu4e-actions ido rfc2368 mu4e-mark mu4e-proc mu4e-utils doc-view jka-compr image-mode exif mu4e-lists mu4e-message flow-fill org ob ob-tangle ob-ref ob-lob ob-table ob-exp org-macro org-footnote org-src ob-comint org-pcomplete org-list org-faces org-entities org-version ob-emacs-lisp ob-core ob-eval org-table ol org-keys org-compat org-macs org-loaddefs cal-menu calendar cal-loaddefs mule-util hl-line mu4e-vars mu4e-meta smtpmail-multi smtpmail sendmail ecomplete completion auto-dictionary flyspell ispell tramp-smb which-key highlight-parentheses company-restclient know-your-http-well http-status-codes http-relations http-methods http-headers restclient company-posframe posframe company pcase yasnippet compile find-func autoload radix-tree lisp-mnt finder-inf mm-archive network-stream url-cache forge-list forge-commands forge-semi forge-bitbucket buck forge-gogs gogs forge-gitea gtea forge-gitlab glab forge-github ghub-graphql treepy gsexp ghub let-alist gnutls forge-notify forge-revnote forge-pullreq forge-issue forge-topic forge-post markdown-mode color thingatpt noutline outline forge-repo forge forge-core forge-db closql emacsql-sqlite advice emacsql emacsql-compiler magit-submodule magit-obsolete magit-blame magit-stash magit-reflog magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote magit-commit magit-sequence magit-notes magit-worktree magit-tag magit-merge magit-branch magit-reset magit-files magit-refs magit-status magit magit-repos magit-apply magit-wip magit-log which-func imenu magit-diff smerge-mode diff diff-mode git-commit log-edit pcvs-util add-log magit-core magit-autorevert autorevert filenotify magit-margin magit-transient magit-process with-editor server magit-mode transient comp comp-cstr magit-git magit-section magit-utils crm dash visual-filename-abbrev debbugs soap-client url-http url-auth url-gw nsm warnings rng-xsd rng-dt rng-util xsd-regexp bug-reference use-package-bind-key bind-key easy-mmode vertico aggressive-completion icomplete mb-depth use-package-diminish windmove alert log4e notifications gntp rx tramp-cache tramp-sh recentf tree-widget pixel-scroll saveplace savehist paren smiley gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-sum shr kinsoku svg dom gnus-group gnus-undo gnus-start gnus-dbus dbus xml gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int gnus-range message rmc puny dired dired-loaddefs rfc822 mml mml-sec epa derived epg epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader gnus-win gnus wid-edit nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums text-property-search mm-util mail-prsvr mail-utils edmacro kmacro dracula-theme diminish cl-extra help-mode use-package-ensure use-package-core tramp tramp-loaddefs trampver tramp-integration files-x tramp-compat shell pcomplete comint ansi-color ring parse-time iso8601 time-date ls-lisp format-spec info package browse-url url url-proxy url-privacy url-expand url-methods url-history url-cookie url-domsuf url-util mailcap url-handlers url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs password-cache json subr-x map url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win term/common-win x-dnd 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 easymenu timer select scroll-bar mouse jit-lock font-lock syntax 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 button loaddefs faces cus-face macroexp files window text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote threads dbusbind inotify lcms2 dynamic-setting system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit x multi-tty make-network-process native-compile emacs) Memory information: ((conses 16 720190 82629) (symbols 48 47153 5) (strings 32 211165 12252) (string-bytes 1 6287106) (vectors 16 84612) (vector-slots 8 1466836 93450) (floats 8 635 676) (intervals 56 4752 453) (buffers 992 30))
> From: Tassilo Horn <tsdh@gnu.org>
> Date: Thu, 13 May 2021 22:45:01 +0200
> Cc: Stefan Kangas <stefan@marxist.se>
>
> Since commit 1af91d271e077134e272055407fb8c4312a7579b I get strange
> "Invalid version syntax: <package> <version> (must start with a number)"
> errors during M-x completion in the minibuffer. I can't reproduce with
> emacs -Q but in the error, <package> is always some (M)ELPA package.
>
> For example, M-x mag TAB gives
>
> completion--some: Invalid version syntax: ‘Magit 3.0.0’ (must start with a number)
That commit uses byte-obsolete-info property of a symbol, and expects
its caddr to be a valid version number. It sounds like in some cases
it is not an Emacs version, but something else. So, questions:
. can you find out what kind of value does the byte-obsolete-info
property is there on Magit functions? and
. how come these properties were added to something that is not an
Emacs core code?
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?
Eli Zaretskii <eliz@gnu.org> writes: >> For example, M-x mag TAB gives >> >> completion--some: Invalid version syntax: ‘Magit 3.0.0’ (must start with a number) > > That commit uses byte-obsolete-info property of a symbol, and expects > its caddr to be a valid version number. It sounds like in some cases > it is not an Emacs version, but something else. So, questions: > > . can you find out what kind of value does the byte-obsolete-info > property is there on Magit functions? and It's not just Magit. I've grepped my ~/.emacs.d/elpa/ and here are some values: (make-obsolete-variable 'magit-refresh-args nil "Magit 3.0.0") (make-obsolete-variable backported sym "yasnippet 0.8") (make-obsolete-variable 'which-key-key-replacement-alist 'which-key-replacement-alist "2016-11-21") So at least magit and yasnippet use "<package> <version>" syntax here, and which-key uses a date (which seems fine for version-to-list). > . how come these properties were added to something that is not an > Emacs core code? It seems to be added by `make-obsolete', no? Otherwise, there is no writing menion of byte-obsolete-info. > 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. 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? Bye, Tassilo
> 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.
> Date: Fri, 14 May 2021 10:18:40 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 48404@debbugs.gnu.org, stefan@marxist.se
>
> 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?
^^^^
Should be "test", sorry. My fingers sometimes think they know better...
Tassilo Horn <tsdh@gnu.org> writes: > I've tried this [...] > - (>= (car (version-to-list (caddr obsolete))) > - emacs-major-version)))))) > + (condition-case nil > + (>= (car (version-to-list (caddr obsolete))) > + emacs-major-version) > + (t nil))))))) Yes, I think this is the right solution. (Or just `ignore-errors'.) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
Eli Zaretskii <eliz@gnu.org> writes: > 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? The idea is to have a softer transition when obsoleting commands -- `M-x some-obsoleteTAB' will list the command if it's been "recently" obsoleted, but also list the new command name (in parentheses). This will hopefully help people to learn the new command name. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
If the check does not work reliably for third-party packages, it should probably better reverted altogether. Why slap an ignore-errors around the broken code now? According to the commit message, which introduced this bug, there has been some alternative solution, which clearly sounds better: Either omit obsolete commands altogether or annotate them as obsolete. Omitting is probably too problematic since people may rely on running obsolete commands. What about making the behavior around obsolete commands a customizable option? 1. omit obsolete commands, 2. annotate obsolete commands, 3. do nothing.
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Tassilo Horn <tsdh@gnu.org>, 48404@debbugs.gnu.org, stefan@marxist.se
> Date: Fri, 14 May 2021 09:43:37 +0200
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > 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?
>
> The idea is to have a softer transition when obsoleting commands -- `M-x
> some-obsoleteTAB' will list the command if it's been "recently"
> obsoleted, but also list the new command name (in parentheses). This
> will hopefully help people to learn the new command name.
But this comes with a heavy price: commands from any package that is
not bundled with Emacs will not be able to take advantage of this
feature, ever. Is it worth punishing those packages' users to have a
softer transition? Given our conservative approach to obsoleting
stuff, I'm not sure.
Eli Zaretskii <eliz@gnu.org> writes:
> > Since commit 1af91d271e077134e272055407fb8c4312a7579b I get strange
> > "Invalid version syntax: <package> <version> (must start with a number)"
> > errors during M-x completion in the minibuffer. I can't reproduce with
> > emacs -Q but in the error, <package> is always some (M)ELPA package.
> >
> > For example, M-x mag TAB gives
> >
> > completion--some: Invalid version syntax: ‘Magit 3.0.0’ (must start with a number)
>
> That commit uses byte-obsolete-info property of a symbol, and expects
> its caddr to be a valid version number. It sounds like in some cases
> it is not an Emacs version, but something else. So, questions:
>
> . can you find out what kind of value does the byte-obsolete-info
> property is there on Magit functions? and
> . how come these properties were added to something that is not an
> Emacs core code?
>
> 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?
The timing for Lars pushing my patch was a bit unfortunate, as I'm
currently away traveling and won't have any time to look at this in
the next week or two at least. I unfortunately didn't say so in the
relevant bug thread, but I hadn't yet convinced myself that the patch
was the correct one, and had the intention of returning to it for a
closer look before pushing.
If any immediate issues can be resolved, this is on my todo list and I
will take a look at this when I'm back home. It would also be fine to
just revert my patch for now and re-open the bug report.
Thanks.
Stefan Kangas <stefan@marxist.se> writes: > If any immediate issues can be resolved, this is on my todo list and I > will take a look at this when I'm back home. It would also be fine to > just revert my patch for now and re-open the bug report. No prob -- I've pushed a fix that should stop `M-x' bugging out on invalid versions strings now. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
Why not revert the broken M-x predicate until a better solution is found? For example as I mentioned adding an option which allows to customize the treatment of obsolete commands (hide, annotate, keep). It looks like the solution we got now is suboptimal: 1. The version check does not work for third-party packages, it does not even work for packages which are part of Emacs (so-long). 2. Since the version check is broken, an `ignore-errors` wrapper is needed. 3. The behavior is not customizable, some users may prefer to hide obsolete commands altogether, some would like to see an annotation and some would like to keep the obsolete commands in M-x. Daniel
Eli Zaretskii <eliz@gnu.org> writes: >> The idea is to have a softer transition when obsoleting commands -- `M-x >> some-obsoleteTAB' will list the command if it's been "recently" >> obsoleted, but also list the new command name (in parentheses). This >> will hopefully help people to learn the new command name. > > But this comes with a heavy price: commands from any package that is > not bundled with Emacs will not be able to take advantage of this > feature, ever. Is it worth punishing those packages' users to have a > softer transition? Given our conservative approach to obsoleting > stuff, I'm not sure. I'm not quite sure I understand you here? If the version can't be parsed, then the command will still show up in `M-x TAB', which means that the obsoletion of these commands is very soft indeed. But I'm wondering whether `version-to-list' should be more lax here. That is, currently it'll barf of things like: (version-to-list "28.1 Magit/2.5") It might make sense to allow the obsoletion versions to refer to both an (approximate) Emacs version, as well as a package version. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
Daniel Mendler <mail@daniel-mendler.de> writes: > 3. The behavior is not customizable, some users may prefer to hide > obsolete commands altogether, some would like to see an annotation and > some would like to keep the obsolete commands in M-x. Yes, it might make sense to allow `read-extended-command-predicate' to be a list of predicates, and this obsoletion predicate might be in that list by default. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
> From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: tsdh@gnu.org, 48404@debbugs.gnu.org, stefan@marxist.se > Date: Sun, 16 May 2021 16:06:03 +0200 > > > But this comes with a heavy price: commands from any package that is > > not bundled with Emacs will not be able to take advantage of this > > feature, ever. Is it worth punishing those packages' users to have a > > softer transition? Given our conservative approach to obsoleting > > stuff, I'm not sure. > > I'm not quite sure I understand you here? If the version can't be > parsed, then the command will still show up in `M-x TAB', which means > that the obsoletion of these commands is very soft indeed. For obsolete commands from 3rd-party packages, which state something like "Magit 3.0" in the version since which they are obsolete, the obsolescence will never happen, in the sense that they will _always_ appear in "M-x TAB", even 100 years from now. Is that what we want? > But I'm wondering whether `version-to-list' should be more lax here. > That is, currently it'll barf of things like: > > (version-to-list "28.1 Magit/2.5") I'm not sure it's possible without introducing ambiguity into the version string and complicating comparison of versions. We already support some non-numeric versions, and that's not easy. > It might make sense to allow the obsoletion versions to refer to both an > (approximate) Emacs version, as well as a package version. How can this be done, even in principle? Versions of unbundled packages are unrelated to Emacs versions; typically, a given version of an unbundled package supports quite a few Emacs versions. So what Emacs version will you put there?
Eli Zaretskii <eliz@gnu.org> writes: > For obsolete commands from 3rd-party packages, which state something > like "Magit 3.0" in the version since which they are obsolete, the > obsolescence will never happen, in the sense that they will _always_ > appear in "M-x TAB", even 100 years from now. Is that what we want? That is what was the case before applying the patch, so it's no change for those commands. >> But I'm wondering whether `version-to-list' should be more lax here. >> That is, currently it'll barf of things like: >> >> (version-to-list "28.1 Magit/2.5") > > I'm not sure it's possible without introducing ambiguity into the > version string and complicating comparison of versions. We already > support some non-numeric versions, and that's not easy. Sorry, I was imprecise here -- I didn't mean that we should change `version-to-list' itself here, but add a new function that's more permissive, just for use in this context. It would basically be something along the lines of (version-to-list (car (split-string string))) >> It might make sense to allow the obsoletion versions to refer to both an >> (approximate) Emacs version, as well as a package version. > > How can this be done, even in principle? Versions of unbundled > packages are unrelated to Emacs versions; typically, a given version > of an unbundled package supports quite a few Emacs versions. So what > Emacs version will you put there? That would be up to the package maintainers -- they get to choose what Emacs version(s) the obsolete command would show up in `M-x TAB' completions. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
> From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: tsdh@gnu.org, 48404@debbugs.gnu.org, stefan@marxist.se > Date: Sun, 16 May 2021 16:38:58 +0200 > > Eli Zaretskii <eliz@gnu.org> writes: > > > For obsolete commands from 3rd-party packages, which state something > > like "Magit 3.0" in the version since which they are obsolete, the > > obsolescence will never happen, in the sense that they will _always_ > > appear in "M-x TAB", even 100 years from now. Is that what we want? > > That is what was the case before applying the patch, so it's no change > for those commands. No change for the worse, sure. That's what I meant when I said they will not be able to take advantage of this feature. They will be excluded. > >> (version-to-list "28.1 Magit/2.5") > > > > I'm not sure it's possible without introducing ambiguity into the > > version string and complicating comparison of versions. We already > > support some non-numeric versions, and that's not easy. > > Sorry, I was imprecise here -- I didn't mean that we should change > `version-to-list' itself here, but add a new function that's more > permissive, just for use in this context. It would basically be > something along the lines of (version-to-list (car (split-string string))) The main purpose is to compare versions. How would you do that with loose version strings such as the one above? > >> It might make sense to allow the obsoletion versions to refer to both an > >> (approximate) Emacs version, as well as a package version. > > > > How can this be done, even in principle? Versions of unbundled > > packages are unrelated to Emacs versions; typically, a given version > > of an unbundled package supports quite a few Emacs versions. So what > > Emacs version will you put there? > > That would be up to the package maintainers -- they get to choose what > Emacs version(s) the obsolete command would show up in `M-x TAB' > completions. I don't think they will be able to solve this problem, not in satisfactory ways anyway.
On 5/16/21 4:09 PM, Lars Ingebrigtsen wrote:
> Daniel Mendler <mail@daniel-mendler.de> writes:
>
>> 3. The behavior is not customizable, some users may prefer to hide
>> obsolete commands altogether, some would like to see an annotation and
>> some would like to keep the obsolete commands in M-x.
>
> Yes, it might make sense to allow `read-extended-command-predicate' to
> be a list of predicates, and this obsoletion predicate might be in that
> list by default.
This sounds like a good resolution of the issue. Thanks!
Daniel
Eli Zaretskii <eliz@gnu.org> writes: >> >> (version-to-list "28.1 Magit/2.5") >> > >> > I'm not sure it's possible without introducing ambiguity into the >> > version string and complicating comparison of versions. We already >> > support some non-numeric versions, and that's not easy. >> >> Sorry, I was imprecise here -- I didn't mean that we should change >> `version-to-list' itself here, but add a new function that's more >> permissive, just for use in this context. It would basically be >> something along the lines of (version-to-list (car (split-string string))) > > The main purpose is to compare versions. How would you do that with > loose version strings such as the one above? We're looking at the Emacs version (which we'd put first in the version string, while the rest is essentially a free text used as a comment). -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tsdh@gnu.org, 48404@debbugs.gnu.org, stefan@marxist.se
> Date: Mon, 17 May 2021 15:54:43 +0200
>
> >> >> (version-to-list "28.1 Magit/2.5")
> >> >
> >
> > The main purpose is to compare versions. How would you do that with
> > loose version strings such as the one above?
>
> We're looking at the Emacs version (which we'd put first in the version
> string, while the rest is essentially a free text used as a comment).
What do you mean by "first"? We already support the likes of
"28.3 Beta3", and "Beta3" is not ignored there as some free-text
comment.
And that is assuming the developers of 3rd-party packages can have
some reasonable way of associating their versions with Emacs versions,
which I still think is a problem with no solution.
Eli Zaretskii <eliz@gnu.org> writes: > What do you mean by "first"? We already support the likes of > "28.3 Beta3", and "Beta3" is not ignored there as some free-text > comment. For this purpose, it doesn't matter -- we're only interested in the major version. But if you want to expand the language in a more regular way, then a different separating character (between the part we parse and the one that's free text) can be used, of course. Semicolon? > And that is assuming the developers of 3rd-party packages can have > some reasonable way of associating their versions with Emacs versions, > which I still think is a problem with no solution. They don't really need to. They decide "we think people with Emacs 25 shouldn't have this in their M-x TAX" and then put "26.1;Magit/1.2" in the string. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
> From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: tsdh@gnu.org, 48404@debbugs.gnu.org, stefan@marxist.se > Date: Mon, 17 May 2021 16:20:25 +0200 > > Eli Zaretskii <eliz@gnu.org> writes: > > > What do you mean by "first"? We already support the likes of > > "28.3 Beta3", and "Beta3" is not ignored there as some free-text > > comment. > > For this purpose, it doesn't matter -- we're only interested in the > major version. But if you want to expand the language in a more regular > way, then a different separating character (between the part we parse > and the one that's free text) can be used, of course. Semicolon? Maybe, I don't know. My point is that simply ignoring something after the first space is not a good idea. > > And that is assuming the developers of 3rd-party packages can have > > some reasonable way of associating their versions with Emacs versions, > > which I still think is a problem with no solution. > > They don't really need to. They decide "we think people with Emacs 25 > shouldn't have this in their M-x TAX" and then put "26.1;Magit/1.2" in > the string. We need to talk to them to get their agreement, I think. We cannot decide for them that this is what they should do.
Eli Zaretskii <eliz@gnu.org> writes: >> They don't really need to. They decide "we think people with Emacs 25 >> shouldn't have this in their M-x TAX" and then put "26.1;Magit/1.2" in >> the string. > > We need to talk to them to get their agreement, I think. We cannot > decide for them that this is what they should do. I think we just have to establish a convention that external packages could use. Whether they then want to use it is up to them. That is, if they don't want to, then Emacs' behaviour with regards to obsoleted commands won't change -- but if they want to see this new behaviour, they'll need to reformat their obsoletion strings. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
>> And that is assuming the developers of 3rd-party packages can have
>> some reasonable way of associating their versions with Emacs
>> versions, which I still think is a problem with no solution.
>
> They don't really need to. They decide "we think people with Emacs 25
> shouldn't have this in their M-x TAX" and then put "26.1;Magit/1.2" in
> the string.
FWIW, as a 3rd-party package maintainer, I've never have the need to
define that some commands should not show up in completion with a
specific emacs version. In auctex, we deal with emacs versions by just
requiring a minimal version (which gets bumpt very conservatively), and
that's about it. But of course, YMMV.
In auctex, all our obsoletion versions are just version numbers "11.97"
like and represent the auctex version and our bug reports show that old
versions are still used much more often as one would expect. So as long
as those "normal" versions don't cause the package version be compared
to the emacs version and just recently deprecated commands vanish from
completion, I'm neutral.
Bye,
Tassilo
> From: Tassilo Horn <tsdh@gnu.org>
> Cc: Eli Zaretskii <eliz@gnu.org>, 48404@debbugs.gnu.org, stefan@marxist.se
> Date: Wed, 19 May 2021 13:54:22 +0200
>
> In auctex, all our obsoletion versions are just version numbers "11.97"
> like and represent the auctex version and our bug reports show that old
> versions are still used much more often as one would expect. So as long
> as those "normal" versions don't cause the package version be compared
> to the emacs version and just recently deprecated commands vanish from
> completion, I'm neutral.
AFAIU, they do get compared with Emacs version numbers.