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

Support SLY as well as SLIME #21

Closed
wants to merge 1 commit into from

Conversation

svetlyak40wt
Copy link

I've modified log4slime that way, that it is now supports SLY.

Actually, I want to support both SLIME and SLY and need an advice about a better way of doing that.

@svetlyak40wt
Copy link
Author

svetlyak40wt commented Dec 3, 2017

@scymtym @dkochmanski @slyrus @knobo I need your advice here.

@scymtym
Copy link
Member

scymtym commented Sep 23, 2018

@svetlyak40wt sorry, completely missed this.

As you point out, we cannot simply rename everything and lose support for SLIME.

One way to support both backends would be adding new functions

(defun backend-eval (form)
  (cond (*using-slime*
          (funcall (intern "slime-eval") form)
        (*using-sly*
          (funcall (intern "sly-eval") form))))

etc. and replace all uses of slime-eval with backend-eval etc. *using-slime* and *using-sly* would be set according to which backend is loaded.

@svetlyak40wt
Copy link
Author

Ok, will try to add such functions.

By the way, I'm now working on a project which should help to not miss any communication in Github issues and pulls. @scymtym are you interested to become a beta tester when the first MVP will be available?

@scymtym
Copy link
Member

scymtym commented Apr 25, 2019

@scymtym are you interested to become a beta tester when the first MVP will be available?

Thanks, but I don't feel the need for something like that at the moment.

@bandoos
Copy link

bandoos commented Jul 20, 2019

@svetlyak40wt do I have to take any particular care to get the colorization under sly?
Thanks in advance

@svetlyak40wt
Copy link
Author

@bandoos no, it just works out of the box. I'm using OSX, iTerm2 and Emacs 26.1

@svetlyak40wt
Copy link
Author

@bandoos sorry, I misinterpreted your question. I don't have colorization for log4cl's output in the SLY :(

But anyway, most of the time, I'm using a wrapper around log4cl: https://github.com/40ants/log4cl-json

hdasch added a commit to hdasch/log4cl that referenced this pull request Aug 19, 2021
This adds support for ‘sly’ to the existing ‘slime’ interface.
Selection of backend (‘slime’ or ‘sly’) occurs at runtime.

If only one or the other of the backend packages is loaded, selection
is simple.  This is expected to be the common case.

If both backend packages are loaded, ‘slime’ is preferred.  To
override this prejudice, ‘customize-variable’ the variable
‘log4slime-backend’.  There is no good way, at present, to support
both packages if both are connected to active common lisp runtimes.
Select one or the other for you logging pleasure.

This is a request for comment.  Feedback is welcome.

In particular, the backends differ in than ‘sly’ turns on ‘font-lock’,
while ’slime’ does not.  To make category highlighting work on ‘sly’
the ‘log4slime-category-*-properties’ variables are let bound for
‘sly’ message formatting.  See the "REVIEW NEEDED" comment.

This minimizes the change to the ‘slime’ interface.  But it may make
sense to simply extend the ‘log4slime-category-*-properties’
definitions.

This change set introduces ‘defalias’ and ’defvaralias’ for
‘log-4slime-mode’, ‘turn-on-log4sly-mode’, and ’global-log4sly-mode’.
This is done to minimize changes existing Sly users who are using the
log4slime.el fork suggested in pull request sharplispers#21 would need to make.

One minor side effect of these aliases is an complaint in the message
buffer if log4slime.el is reevaluated.  The ‘defvaralias’ ‘for
log4sly-mode’ produces the message "Don’t know how to make a localized
variable an alias".  It is not clear how to fix this other than
removing the ‘defvaralias’.  But the message is benign.
hdasch added a commit to hdasch/log4cl that referenced this pull request Aug 23, 2021
This adds support for ‘sly’ to the existing ‘slime’ interface.
Selection of backend (‘slime’ or ‘sly’) occurs at runtime.

If only one or the other of the backend packages is loaded, selection
is simple.  This is expected to be the common case.

If both backend packages are loaded, ‘slime’ is preferred.  To
override this prejudice, ‘customize-variable’ the variable
‘log4slime-backend’.  There is no good way, at present, to support
both packages if both are connected to active common lisp runtimes.
Select one or the other for you logging pleasure.

This is a request for comment.  Feedback is welcome.

In particular, the backends differ in than ‘sly’ turns on ‘font-lock’,
while ’slime’ does not.  To make category highlighting work on ‘sly’
the ‘log4slime-category-*-properties’ variables are let bound for
‘sly’ message formatting.  See the "REVIEW NEEDED" comment.

This minimizes the change to the ‘slime’ interface.  But it may make
sense to simply extend the ‘log4slime-category-*-properties’
definitions.

This change set introduces ‘defalias’ and ’defvaralias’ for
‘log-4slime-mode’, ‘turn-on-log4sly-mode’, and ’global-log4sly-mode’.
This is done to minimize changes existing Sly users who are using the
log4slime.el fork suggested in pull request sharplispers#21 would need to make.

One minor side effect of these aliases is an complaint in the message
buffer if log4slime.el is reevaluated.  The ‘defvaralias’ ‘for
log4sly-mode’ produces the message "Don’t know how to make a localized
variable an alias".  It is not clear how to fix this other than
removing the ‘defvaralias’.  But the message is benign.
@hdasch hdasch mentioned this pull request Aug 27, 2021
@scymtym
Copy link
Member

scymtym commented Oct 21, 2021

@svetlyak40wt Does #40 do everything you wanted to achieve with this pull request?

@svetlyak40wt
Copy link
Author

Probably, #40 solves my problem. I'll give it a try. Thank you!

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.

None yet

3 participants