Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symex interface #136

Merged
merged 10 commits into from
Jun 13, 2024
Merged

Symex interface #136

merged 10 commits into from
Jun 13, 2024

Conversation

pbaille
Copy link
Contributor

@pbaille pbaille commented Jun 6, 2024

Summary of Changes

Following the discussion about symex interface extension, I leveraged the new symex-interface-extend construct to register builtin interfaces.

This is an experiment, mostly here to trigger discussions.

Public Domain Dedication

  • In contributing, I relinquish any copyright claims on my contribution and freely release it into the public domain in the simple hope that it will provide value.

(Why: The freely released, copyright-free work in this repository represents an investment in a better way of doing things called attribution-based economics. Attribution-based economics is based on the simple idea that we gain more by giving more, not by holding on to things that, truly, we could only create because we, in our turn, received from others. As it turns out, an economic system based on attribution -- where those who give more are more empowered -- is significantly more efficient than capitalism while also being stable and fair (unlike capitalism, on both counts), giving it transformative power to elevate the human condition and address the problems that face us today along with a host of others that have been intractable since the beginning. You can help make this a reality by releasing your work in the same way -- freely into the public domain in the simple hope of providing value. Learn more about attribution-based economics at drym.org, tell your friends, do your part.)

Comment on lines 86 to 96
(symex-interface-extend
symex-arc-modes
(list
:eval #'symex-eval-arc
:eval-definition #'symex-eval-definition-arc
:eval-pretty #'symex-eval-pretty-arc
:eval-thunk #'symex-eval-thunk-arc
:eval-print #'symex-eval-print-arc
:describe-symbol #'symex-describe-symbol-arc
:repl #'symex-repl-arc
:run #'symex-run-arc))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be wrap those forms into thunks ?
That may be called in the symex-interface-builtins.el (which would justify it a little more)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you see any benefits from wrapping these in thunks? Otherwise, it seems fine as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thunk wrapping could bring some flexibility maybe ?
Maybe there is some subtelties that I don't get though.
Any concrete thoughts you have on this ?

Copy link
Collaborator

@countvajhula countvajhula Jun 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean something like this:

:eval (lambda () (symex-eval-arc))

I can't think of any reason to do this, so let's leave it as is. If we find we need more flexibility in the future we can always add what we need then. "YAGNI," as they say.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sorry I confused my self, I took your answer for a comment, forgetting about my initial question...
Ok :) so let's keep it like it is.
I was talking about wrapping the whole symex-interface-extend form into a thunk.

(defun start-symex-arc ()
  (symex-interface-extend
   symex-arc-modes
   (list
    :eval #'symex-eval-arc
    :eval-definition #'symex-eval-definition-arc
    :eval-pretty #'symex-eval-pretty-arc
    :eval-thunk #'symex-eval-thunk-arc
    :eval-print #'symex-eval-print-arc
    :describe-symbol #'symex-describe-symbol-arc
    :repl #'symex-repl-arc
    :run #'symex-run-arc)))

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, that's a good point. I am also uneasy about having things happen at module loading time instead of via an explicit function invocation at the appropriate time. Are you thinking something like this:

symex-interface-arc.el:

(defun symex-interface-register-arc ()
  "Register the Arc runtime interface."
  (symex-interface-extend
   symex-arc-modes
   (list
    :eval #'arc-send-last-sexp
    :eval-definition #'arc-send-definition
    :eval-pretty #'arc-send-last-sexp
    :eval-thunk #'symex-eval-thunk-arc
    :repl #'symex-repl-arc
    :switch-to-scratch-buffer #'symex-switch-to-scratch-buffer-arc)))

symex-interface-builtins.el:

(defun symex-register-builtin-interfaces ()
  "Register built-in runtime interfaces."
  (symex-interface-register-arc)
  ...)

And then symex-register-builtin-interfaces could be called in symex-initialize.

Yes, I think something along these lines would be a good idea.

symex-misc.el Show resolved Hide resolved
Copy link
Contributor Author

@pbaille pbaille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some thoughts about the first iteration.

Copy link
Collaborator

@countvajhula countvajhula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm liking these a lot. Made a few comments.

symex-interface-clojure.el Outdated Show resolved Hide resolved
symex-interface-common-lisp.el Outdated Show resolved Hide resolved
symex-interface-elisp.el Outdated Show resolved Hide resolved
symex-interface.el Outdated Show resolved Hide resolved
symex-interface.el Show resolved Hide resolved
symex.el Show resolved Hide resolved
symex-misc.el Show resolved Hide resolved
Comment on lines 86 to 96
(symex-interface-extend
symex-arc-modes
(list
:eval #'symex-eval-arc
:eval-definition #'symex-eval-definition-arc
:eval-pretty #'symex-eval-pretty-arc
:eval-thunk #'symex-eval-thunk-arc
:eval-print #'symex-eval-print-arc
:describe-symbol #'symex-describe-symbol-arc
:repl #'symex-repl-arc
:run #'symex-run-arc))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you see any benefits from wrapping these in thunks? Otherwise, it seems fine as is.

symex-interface-builtins.el Show resolved Hide resolved
symex-interface.el Outdated Show resolved Hide resolved
symex-interface-arc.el Outdated Show resolved Hide resolved
@countvajhula
Copy link
Collaborator

@pbaille Are you planning on adding switch-to-scratch-buffer to the methods interface? (ref. this comment) That's the main remaining item if I am not mistaken.

@pbaille
Copy link
Contributor Author

pbaille commented Jun 9, 2024

@pbaille Are you planning on adding switch-to-scratch-buffer to the methods interface? (ref. this comment) That's the main remaining item if I am not mistaken.

Yes it seems to be the last thing to do, I will take care of it now :)
Thank you a lot for the careful review.

@countvajhula
Copy link
Collaborator

@dcostaras @anonimitoraf @markgdawson @doyougnu If you have a moment, we could use your help testing that the interface to Clojure, ClojureScript, Common Lisp SLIME and SLY still work as expected.

To test, you could follow these steps:

  1. Clone @pbaille 's fork and switch to the symex-interface branch
  2. Restart Emacs for Symex to be built with the new changes
  3. Try out e, d, r, t etc. in your favorite flavor of Lisp.
  4. Report back here

Thanks!

@pbaille pbaille marked this pull request as ready for review June 9, 2024 16:58
@anonimitoraf
Copy link
Contributor

Hey @countvajhula, I'm going to be writing a bit of Clojure code soon, so will report back!

@pbaille
Copy link
Contributor Author

pbaille commented Jun 10, 2024

On my side, I tried clojure, elisp and fennel.

@anonimitoraf
Copy link
Contributor

Clojure seems fine! Btw, I'm using symex-hydra. Does that matter?

@pbaille
Copy link
Contributor Author

pbaille commented Jun 10, 2024

thank you @anonimitoraf, it should not matter, the same actions are called hydra or not AFAICT.

@countvajhula
Copy link
Collaborator

Tested on ELisp and Racket, and LGTM. The only thing I noticed is that Rigpa uses symex-lisp-modes and was failing to load as a result. I wouldn't really consider this variable to be a supported public interface though. I will push a fix to Rigpa to use (symex-get-lisp-modes).

Merging. Thank you @pbaille ! And also thanks @anonimitoraf for testing!

@countvajhula countvajhula merged commit f1588bd into drym-org:master Jun 13, 2024
1 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants