Hi,

In Org mode, we have recently encountered a bug in
`combine-change-calls' macro. See
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=60467

If we want to use `combine-change-calls' in the earlier versions of
Emacs, it may or may not be a good idea to use the buggy old version.

The usual solution to this problem we use in Org is defining a custom
`org-combine-change-calls' that will either provide a fixed version of
the macro for older Emacs or simply implement a stub macro that will not
trigger the bug in old Emacs at the expense of degraded functionality.

If we were to use compat.el, we would ideally want it to provide the
fixed version.

However, it is not always possible to fix buggy functions. For example,
subroutines cannot be easily fixed. Yet, I would find it useful if
compat.el still provided some stub versions of functions, even as simple
as (lambda (&rest _) nil). Or, better, allowed defining custom
extensions of compat.el functions depending on Emacs version. Then,
hypothetical macro (compat-call-for 'org fun args) would select the
special function definition defined by 'org for matching Emacs version.

WDYT?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Hi Ihor!

> The usual solution to this problem we use in Org is defining a custom
> `org-combine-change-calls' that will either provide a fixed version of
> the macro for older Emacs or simply implement a stub macro that will not
> trigger the bug in old Emacs at the expense of degraded functionality.
>
> If we were to use compat.el, we would ideally want it to provide the
> fixed version.

The macro `combine-change-calls' has been introduced in Emacs 27, so we
could in principle provide it as part of Compat to also support Emacs 26
and older. On Emacs 27 (where the macro has bugs), you could use
`compat-call' to access the fixed version.

However I've recently looked into `combine-change-calls' (when we first
talked about Org and Compat), and I am not overly optimistic that this
macro can be backported easily and robustly given its complexity and
dependence on undo internals. Unfortunately Emacs lacks tests, so we
cannot just copy the macro and tests over. Maybe using a stub with
degraded functionality would be the way to go in this case?

> However, it is not always possible to fix buggy functions. For example,
> subroutines cannot be easily fixed. Yet, I would find it useful if
> compat.el still provided some stub versions of functions, even as simple
> as (lambda (&rest _) nil). 

It is justified to add stubs to Compat, if they at least provide some
degraded functionality. As an example, the recently added
`with-delayed-message' macro is essentially a stub. It still evaluates
the BODY but does not implement the side effect of printing a message.

Could you be more specific regarding the kind of stubs you would want to
add? A `combine-change-calls' macro which does nothing except evaluating
the BODY may fall within the category of reasonable stubs, since the
observable behavior will still be mostly intact except for the change
calls. Is this mostly an efficiency problem or would we expect other
downsides if the change calls are not combined? To be clear, I would
rather lean on the safe side and not provide something if we are not
100% sure.

I've recently written down some criteria for the functionality provided
by Compat. See
https://elpa.gnu.org/packages/doc/compat.html#Limitations. These
criteria will ensure that Compat stays within reasonable bounds and
stays maintainable. Most importantly Compat must not jeopardize Emacs
stability. That's the reason why we avoid advices and don't overwrite
existing definitions. Please let me know if you have comments regarding
the limitations.

> Or, better, allowed defining custom
> extensions of compat.el functions depending on Emacs version. Then,
> hypothetical macro (compat-call-for 'org fun args) would select the
> special function definition defined by 'org for matching Emacs version.

How exactly is the proposed `compat-call-for' macro supposed to work?
Note that you can (and should) replicate `compat-call' as part of Org,
as long as `compat-call' is not part of `subr-x' or `subr'. See how it
is done in ERC:

https://gitlab.com/emacs-erc/edge/-/blob/e377a2750a8408fecfe0534cf6284b64cd263203/erc-compat.el#L46-76

You could then implement your own variant `org-compat-call` which first
tries to resolve `org-compat--<fun>', then `compat--<fun>' and finally
`<fun>'. Is this what you have in mind?

Daniel

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

> However I've recently looked into `combine-change-calls' (when we first
> talked about Org and Compat), and I am not overly optimistic that this
> macro can be backported easily and robustly given its complexity and
> dependence on undo internals. Unfortunately Emacs lacks tests, so we
> cannot just copy the macro and tests over.

Emacs does have tests in general. May you elaborate?

> ...  Maybe using a stub with
> degraded functionality would be the way to go in this case?

For Org, a stub will do.

> Could you be more specific regarding the kind of stubs you would want to
> add? A `combine-change-calls' macro which does nothing except evaluating
> the BODY may fall within the category of reasonable stubs, since the
> observable behavior will still be mostly intact except for the change
> calls. Is this mostly an efficiency problem or would we expect other
> downsides if the change calls are not combined? To be clear, I would
> rather lean on the safe side and not provide something if we are not
> 100% sure.

Some code may rely on change calls to be combined. Org code does not,
which is why we are able to have

;; lisp/org-compat.el
(unless (fboundp 'combine-change-calls)
  ;; A stub when `combine-change-calls' was not yet there.
  (defmacro combine-change-calls (_beg _end &rest body)
    (declare (debug (form form def-body)) (indent 2))
    `(progn ,@body)))

(I guess it is too aggressive here; we should better provide
`org-combine-change-calls' instead)

I do not think that `combine-change-calls' in particular can cause
troubles (other than performance) when declared as a stub. At the end,
it is generally expected that arbitrary code can arbitrarily modify the
buffer calling before/after-change-functions as needed.

> I've recently written down some criteria for the functionality provided
> by Compat. See
> https://elpa.gnu.org/packages/doc/compat.html#Limitations. These
> criteria will ensure that Compat stays within reasonable bounds and
> stays maintainable. Most importantly Compat must not jeopardize Emacs
> stability. That's the reason why we avoid advices and don't overwrite
> existing definitions. Please let me know if you have comments regarding
> the limitations.

Everything sounds reasonable.

> You could then implement your own variant `org-compat-call` which first
> tries to resolve `org-compat--<fun>', then `compat--<fun>' and finally
> `<fun>'. Is this what you have in mind?

Yes.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

On 2/9/23 12:40, Ihor Radchenko wrote:
>> However I've recently looked into `combine-change-calls' (when we first
>> talked about Org and Compat), and I am not overly optimistic that this
>> macro can be backported easily and robustly given its complexity and
>> dependence on undo internals. Unfortunately Emacs lacks tests, so we
>> cannot just copy the macro and tests over.
> 
> Emacs does have tests in general. May you elaborate?

It does not have a test for this particular `combine-change-calls'
macro. If a function has a test in Emacs, of course we copy the test
over to compat-tests.el. In this case we would have to come up with our
own tests, in particular if we don't only provide a stub, which wouldn't
require much testing.

> Some code may rely on change calls to be combined. Org code does not,
> which is why we are able to have
> 
> ;; lisp/org-compat.el
> (unless (fboundp 'combine-change-calls)
>   ;; A stub when `combine-change-calls' was not yet there.
>   (defmacro combine-change-calls (_beg _end &rest body)
>     (declare (debug (form form def-body)) (indent 2))
>     `(progn ,@body)))
> 
> (I guess it is too aggressive here; we should better provide
> `org-combine-change-calls' instead)

Yes, at least if one wants to be cautious.

> I do not think that `combine-change-calls' in particular can cause
> troubles (other than performance) when declared as a stub. At the end,
> it is generally expected that arbitrary code can arbitrarily modify the
> buffer calling before/after-change-functions as needed.

Probably not, but that's a bet we don't have to take necessarily in
Compat. My suggestion would be to for now go with the
`org-combine-change-calls' stub, since there you explicitly control all
the callsites and you are sure that nothing goes wrong.

Generally Compat works best in the obvious and simple cases, where we
don't expect any breakage due to provided functionality. For example
macros like `defvar-keymap' are safe to use. The same applies to newly
added string utilities. These functions provide safe and immediate value
for downstream packages. It helps in reducing the amount of
compatibility code we have to write in packages, but it does not
necessarily reduce it to zero. The goals of Compat are modest, but from
my experience it is already worth it and better than going with a more
risky approach.

More problematic backports, e.g., related to bugs require more
consideration, but we also don't have to solve all these now. There is
nothing wrong with going with the Org-specific stub. In another mail, I
mentioned to you the bug in `minibuffer-with-setup-hook', which I have
to work around in my Consult package, which comes with its own
`consult--minibuffer-with-setup-hook'. While it does not look super
clean, there is technically nothing wrong with that approach.

>> You could then implement your own variant `org-compat-call` which first
>> tries to resolve `org-compat--<fun>', then `compat--<fun>' and finally
>> `<fun>'. Is this what you have in mind?
> 
> Yes.

Okay, my suggestion would then be to implement your own
`org-compat-call' macro which can do such an extended lookup, if it
makes your live easier.

Does my mail answer your questions? I am not sure if I addressed your
feature request properly. Do you have further questions or feedback?

Daniel

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

>> Emacs does have tests in general. May you elaborate?
>
> It does not have a test for this particular `combine-change-calls'
> macro. If a function has a test in Emacs, of course we copy the test
> over to compat-tests.el. In this case we would have to come up with our
> own tests, in particular if we don't only provide a stub, which wouldn't
> require much testing.

https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b2fda50178b8151c3fe707d1a1b6b11f0c1eca12

> Okay, my suggestion would then be to implement your own
> `org-compat-call' macro which can do such an extended lookup, if it
> makes your live easier.
>
> Does my mail answer your questions? I am not sure if I addressed your
> feature request properly. Do you have further questions or feedback?

Yes.
I'd say that the only suggestion would be to document that sometimes it
is necessary to implement own version of `org-compat-call'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

On 2/10/23 12:57, Ihor Radchenko wrote:
> Daniel Mendler <mail@daniel-mendler.de> writes:
> 
>>> Emacs does have tests in general. May you elaborate?
>>
>> It does not have a test for this particular `combine-change-calls'
>> macro. If a function has a test in Emacs, of course we copy the test
>> over to compat-tests.el. In this case we would have to come up with our
>> own tests, in particular if we don't only provide a stub, which wouldn't
>> require much testing.
> 
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b2fda50178b8151c3fe707d1a1b6b11f0c1eca12

Thanks! I wonder why I hadn't seen these tests before when grepping the
code base. But still, the tests are probably insufficient for me since
they don't test if and how often the change functions are called.

I just saw that you added the simple stub prefixed with `org-'. For now
I think it is good like that.
https://git.savannah.gnu.org/cgit/emacs/elpa.git/commit/?h=externals-release/org&id=af1bb1b06a374abca941d141be25bcabc7221df6

>> Okay, my suggestion would then be to implement your own
>> `org-compat-call' macro which can do such an extended lookup, if it
>> makes your live easier.
>>
>> Does my mail answer your questions? I am not sure if I addressed your
>> feature request properly. Do you have further questions or feedback?
> 
> Yes.
> I'd say that the only suggestion would be to document that sometimes it
> is necessary to implement own version of `org-compat-call'.

Okay, sounds good. I will add some documentation and links to the
implementations used in ERC and Org, such that developers can take a
look how Compat is used in other core packages. Maybe I will also add
some links to ELPA packages which demonstrate the usage.

But first things first - I will wait until Org actually starts to use
Compat. I am a bit busy for the next few weeks and I must admit, I am
currently not bored enough to create the trivial search and replace
patch for Org. I may still come to do it eventually as some kind of work
meditation. :)

Daniel

On 2/10/23 13:22, Daniel Mendler wrote:
>>> Okay, my suggestion would then be to implement your own
>>> `org-compat-call' macro which can do such an extended lookup, if it
>>> makes your live easier.
>>>
>>> Does my mail answer your questions? I am not sure if I addressed your
>>> feature request properly. Do you have further questions or feedback?
>>
>> Yes.
>> I'd say that the only suggestion would be to document that sometimes it
>> is necessary to implement own version of `org-compat-call'.
> 
> Okay, sounds good. I will add some documentation and links to the
> implementations used in ERC and Org, such that developers can take a
> look how Compat is used in other core packages. Maybe I will also add
> some links to ELPA packages which demonstrate the usage.

I expanded the basic documentation slightly:

https://github.com/emacs-compat/compat/commit/15d19d0e01768189d94b585f150a430df248b410

Hopefully the situation will improve in the future, where we will get a
minimal version of compat.el within Emacs core itself. There is the
difficulty that this core Compat would then have to require
`compat-<VERSION>' where VERSION is from the future. I could require
`compat-future' with noerror, which is only provided by the Compat
package itself. Another alternative would be to simply add compat-call
and compat-function to subr.el. But before starting any such
discussions, Compat must be adopted by core packages, such that there is
a point in even discussing this.

Daniel

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

> Hopefully the situation will improve in the future, where we will get a
> minimal version of compat.el within Emacs core itself. There is the
> difficulty that this core Compat would then have to require
> `compat-<VERSION>' where VERSION is from the future. I could require
> `compat-future' with noerror, which is only provided by the Compat
> package itself. Another alternative would be to simply add compat-call
> and compat-function to subr.el. But before starting any such
> discussions, Compat must be adopted by core packages, such that there is
> a point in even discussing this.

As you have seen from the poll, Org is going for it. So, you can already
claim that core packages will need compat.

Note that adding org-compat-call will also go into Emacs core as Org is
distributed with Emacs. So, may as well simply add to subr.el directly.

I think you can start emacs-devel discussion already. I will support.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>