bug#66554: [PATCH] Add the public API of Compat to the core

[Philip Kaludercic]

(Sorry for the late response.)

Daniel Mendler <mail@daniel-mendler.de> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
> [...]
>
>>> Philip, do you plan to submit a new version of the patch or do you
>>> want
>>> me to update the patch with a more extensive explanation? We should
>>> keep
>>> in mind that the information we add to the Emacs compat.el cannot be
>>> self sufficient. Emacs developers who want to use Compat must consult
>>> the Compat manual, since that's the place where we document the
>>> available compatibility definitions. Therefore referring to the manual
>>> for further details should be okay, as long as the general mechanism
>>> (and the versioning) is explained sufficiently well in the
>>> commentary of
>>> the compat.el file in Emacs.
>>
>> I have tried to update the patch to clarify some of the points in the
>> discussion, but feel free to change anything you think ought to be
>> changed:
>
> Thank you, Philip. I added a few comments below, mostly about some
> details of the wording. Eli, Stefans, do you think the level of
> information provided in the patch is sufficient?
>
>> diff --git a/doc/lispref/package.texi b/doc/lispref/package.texi
>> index 6f52a33d194..b9239521d33 100644
>> --- a/doc/lispref/package.texi
>> +++ b/doc/lispref/package.texi
>
> [...]
>
>> +The versioning of Compat follows that of Emacs, so one can implicitly
>> +declare what range of Emacs versions a package supports like so:
>> +
>> +@example
>> +;; Package-Requires: ((emacs "27.2") (compat "29.1"))
>> +@end example
>
> The word "range" is misleading. It sounds as if the package supports
> 27.2 to 29.1, while in it actually supports 27.2 and newer and relies on
> some 29.1 APIs.

Changed this.

> [...]
>
>> diff --git a/etc/NEWS b/etc/NEWS
>> index a1874313502..46859d75aac 100644
>> --- a/etc/NEWS
>> +++ b/etc/NEWS
>> @@ -1321,6 +1321,17 @@ This minor mode generates the tags table
>> automatically based on the
>>  current project configuration, and later updates it as you edit the
>>  files and save the changes.
>>  
>> ++++
>> +** New package Compat
>> +The Compat package on GNU ELPA provides forwards-compatibility
>> +support, so that packages that still provide support for older
>> +versions of Emacs can still make use of newer definitions that can be
>> +reasonably re-implemented in Elisp.  Now a "pseudo" Compat package is
>> +part of Emacs, that doesn't provide any compatibility support, but
>> +only implements the public-facing API of Compat so that core packages
>> +can use Compat, while also preventing the installation of Compat on
>> +the most recent version of Emacs.
>
> This NEWS entry explains the addition well, but it is a bit verbose
> compared to other entries. I am not fond of the quoted word "pseudo".
> Maybe say stub of Compat?

Abbreviated it.

>>  * Incompatible Lisp Changes in Emacs 30.1
>>  
>> diff --git a/lisp/emacs-lisp/compat.el b/lisp/emacs-lisp/compat.el
>> new file mode 100644
>> index 00000000000..2882974cf2d
>> --- /dev/null
>> +++ b/lisp/emacs-lisp/compat.el
>> @@ -0,0 +1,94 @@
>> +;;; compat.el --- Pseudo-Compatibility for Elisp -*-
>> lexical-binding: t; -*-
>
> Instead of "Pseudo" maybe write "Stub of the Emacs Lisp Compatibility
> Library"? We could also use "Emacs Lisp Compatibility Library" like in
> ELPA Compat package, since for packages using the Compat library it
> should not make a difference if the builtin compat.el stub or the ELPA
> package is used.

I changed it to "Stub of the Compatibility Library", to avoid an
overlong line and also because Elisp should be implicit.

>> +;;; Commentary:
>
> [...]
>
>> +;; Note that Compat is NOT a core package and this file is NOT
>> +;; available on GNU ELPA.
>
> I find this sentence a bit confusing. What you want to tell here is that
> the compat.el file in Emacs differs from the compat.el file in the ELPA
> package. This is maybe already clear from the other comments so we can
> as well remove this sentence?

Done.

> [...]
>
>> +;;;; Clever trick to avoid installing Compat if not necessary
>> +
>> +;; The versioning scheme of the Compat package follows that of Emacs,
>> +;; to indicate what version of Emacs is being supported.  For example,
>                                                  ^^^^^^^^^
>
> Should we say "supported" here? Compat supports other version than the
> one specified. Compat "provides" functionality from that Emacs version.

Rephrased this, but sounds a bit clunky.

>> +;; the Compat version number 29.2.3.9 would attempt to provide
>> +;; compatibility definitions up to Emacs 29.2, while also designating
>> +;; that this is the third major release and ninth minor release of
>> +;; Compat, for the specific Emacs release.
>> +
>> +;; The package version of this file is specified programmatically,
>> +;; instead of giving a fixed version in the header of this file.  This
>> +;; is done to ensure that the version of compat.el provided by Emacs
>> +;; is always corresponds to the current version of Emacs.  In addition
>       ^^
>
> The "is" should be removed.

Right.

>> +;; to the major-minor version, a large "major release" makes sure that
>> +;; the built-in version of Compat is always preferred over an external
>> +;; installation.  This means that if a package specifies a dependency
>> +;; on Compat which matches the current version of Emacs that is being
>                               ^^^^^^^^^^^^^^^^^^^
>                               
> ...which matches the current *or an older version* of Emacs...

Ok.

>> +;; used, no additional dependencies have to be downloaded.
>> +;;
>> +;; Further details and background on this file can be found in the
>> +;; bug#66554 discussion.
>> +
>> +;;;###autoload (push (list 'compat
>> +;;;###autoload             emacs-major-version
>> +;;;###autoload             emacs-minor-version
>> +;;;###autoload             1.0e+INF)
>> +;;;###autoload       package--builtin-versions)
>
> I prefer if we use 9999 here instead of 1.0e+INF. While infinity is
> semantically correct, the float may lead to problems and hurt
> readability in the package list.

It seems the main issue is that 30.0.1.0e+INF can be displayed without
any problems, but version-to-list rejects it.  Sad, because I think this
is a neat idea but I've changed it to 9999.

>> +(provide 'compat)
>> +;;; compat.el ends here
>
> Daniel

0001-Add-the-public-API-of-Compat-to-the-core.patch
Description: Text Data