Capturing multiple outputs: compatibility with a possible upcoming Org-mode update

[akater]

I'm not using SLY. However, I recently submitted a patch to Org-mode and a patch to SLIME that might be relevant to SLY developers.

In this branch of my org-mode fork, I introduce trace and error output to org-babel Common Lisp blocks. The patched function org-babel-execute:lisp uses sly-eval in some cases. I don't think my patch would break anything for SLY users but I probably should announce this (hopefully) upcomung change here in advance. If SLY uses swank:eval-and-grab-output then something might need to be updated to follow the (hopefully) upcoming Org update.

This pull request to SLIME allows to grab multiple outputs to provide the feature in question. It also makes swank:eval-and-grab-output return more structured output (alist). I'm not sure if SLY follows SLIME releases but then again, this is a possible upcoming change.

Last but not least, even though one SLIME developer agreed to accept the feature some weeks ago, for now it's stalled. My Org patch had not been reviewed. This might be due to pandemic, or because my writing is particully obnoxious, or because the community of users of both SLIME and ob-lisp is small, or due to a combination of reasons. As far as I know, SLY is generally more insterested in new features than SLIME so if you are more interested in implementing or testing this, you're welcome. Anyway, please notify if getting this feature into Org implies your codebase will need an update too. I'm trying to make sure it goes as smooth as possible for end-users.

[joaotavora]

Thanks. I followed the conversation between you and @luismbo and must say I didn't understand half of it. Probably didn't follow very carefully. SLY uses swank:eval-and-grab-output different functions but same protocol protocol. I don't use org-mode much, but I know many people who do, and with SLY.

Now, you seem to be changing the protocol API between Org and SLIME. That's OK, as long as the change is "backwards compatible". What do I mean by this? That you should be able to confirm the veracity of each one of the following statements:

1. people using old SLIME and new Org are not adversely affected;
2. people using old Org and new SLIME are not adversely affected;
3. people using the new SLIME and the new Org are positively affected (I have no idea what's the benefit, but that's beside the point)

If you're in a condition to answer "yes" to each of these three statements, then everything should be fine. I can choose when and how to implement the extended API in SLY, if ever. If you answer "no" to 1 or 2, I'm pretty sure you'll hear from very annoyed SLIME and SLY users about bugs. Pinging @luismbo about this so we can all converse about the backward-compatibility of these changes.

[akater]

Thank you. “Yes” to 1 and 2, there are safety checks for backwards compatibility, to be lifted in some future versions of Org and SLIME. If that's enough for SLY, the issue can be safely closed.

[joaotavora]

"Yes” to 1 and 2, there are safety checks for backwards compatibility

That's encouraging. Can you point to exactly what these safety checks are? I need to know if they are in Org or SLIME and/or predicated on SLIME implementation details (like, say, its specific versioning schemeor the availability of a specific swank symbol).

[akater]

Both in Org and SLIME.

1. SLIME

In SLIME, swank:eval-and-grab-output accepts optional argument now that specifies which outputs to grab: the signature changes from

(string)

to

(string &optional (targets-to-capture '(*standard-output* values)
                                          targets-provided-p))

Note the targets-provided-p. New interface is to return alist. Old interface is to return a list of two elements. We check targets-provided-p and if it's nil, we conclude the old interface is used, and return old-style list.

This check can certainly be dropped when Emacs that ships old Org-mode becomes unsupported. Org moves quite fast; org+slime usage is quite niche; so I don't think maintainers will wait that long if they accept the patch. It can likely be dropped once Org-mode versions missing support for new interface become unsupported. So maybe 9.4, even. I have not received a response yet.

All dependent functions in SLIME get optional argument of the same type as above that defaults to the value that reproduces their existing behaviour. No checks are needed there.

2. Org-mode

In Org, it's

(defalias 'org-babel-execute:lisp
  (if (and (featurep 'slime)
           (version< slime-version "2.25"))
      'org-babel-execute:lisp--legacy
    'org-babel-execute:lisp--multiple-targets-support))

which can be dropped after SLIME versions missing the feature (presumably 2.25, but I didn't have the response yet, as well) become unsupported.

[joaotavora]

(defalias 'org-babel-execute:lisp
  (if (and (featurep 'slime)
           (version< slime-version "2.25"))
      'org-babel-execute:lisp--legacy
    'org-babel-execute:lisp--multiple-targets-support))

So if there's no slime you're calling org-babel-execute:lisp--multiple-targets-support but SLY doesn't have that support. So you will break it no?

[joaotavora]

So you will break it no?

You need to use the legacy function also if no slime is supported. Also, I'm pretty sure there's a compilation warning there (slime-version will not necessarily be defined).

[akater]

Oh right, sly check should be made. I knew I forgot something. It'll be (or slime-exists-and-it-s-old sly-exists-and-it-s-old) instead of just the slime check. So the plan should be

• merge into SLIME, settle on which version v ships the feature
• get green light from Org devs
• figure out which SLY version to check, based on v
• add the check to the Org patch
• merge into Org.

Correct?

When neither SLIME nor SLY are present, there's no need to use legacy version: ob-lisp is simply non-functional then, and it's more natural to presume new interface if someone wants to write an alternative for SLIME and SLY to use Org-mode with.

[akater]

Since we need to revert to legacy behavior when an old version of SLIME or SLY is used, it's better to check this in the body of org-babel-execute:lisp and get rid of defalias. It might blow up the function, so it might look ugly but if it's the right thing, I better do that. Thanks for your comments.

[joaotavora]

@akater aren't you overcomplicating? Why not just:

(defalias 'org-babel-execute:lisp
  (if (or (not (featurep 'slime))
           (version< slime-version "2.25"))
      'org-babel-execute:lisp--legacy
    'org-babel-execute:lisp--multiple-targets-support))

Please don't do any SLY-specific version checking. Touch as little code as possible

[akater]

There's a more subtle case that I did not think about until today: a user might have both SLIME and SLY installed, and alter between them in their Org usage in a single Emacs session. Which one is actually used is only determined when org-babel-execute:lisp is run. (!) So I'm afraid there has to be this finer check at runtime.

[joaotavora]

a user might have both SLIME and SLY installed, and alter between them in their Org usage in a single Emacs session.

Right, and those users probably deserve the punishment. I'm kidding of coruse: a simple way is to error in org-babel-execute:lisp--multiple-targets-support if org-babel-lisp-eval-fn is anything but slime-eval. Or even make it call the legacy function if it's sly-eval. Lots of simple ways to do this:so do what you have to do, but don't break anything, please :-). Maybe test with SLY briefly?

[akater]

Yes, I'll test with SLY. You see, when I tried to use it, it consistently crashed my Emacs hard (I think I had to press Power button) and I never submitted an issue. It only happened on symbol completion, though. Org probably will be fine.

[joaotavora]

Ahah. Well come to a conclusion and remember to press "Send" before you press the power button!