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

feature: add impl-source-read-only editor-variable #1656

Closed

Conversation

sakurawald
Copy link
Contributor

To close issue: #1615

This option is enabled by default, to disable it, eval the form in init.lisp

(setf (lem:variable-value 'lem-core::impl-source-read-only :global) nil)

The typical value of source dir for sbcl implementation are:

  • /usr/share/sbcl-source/src/code/ (installed via yay -S sbcl)
  • ~/.roswell/src/sbcl-2.4.10/src/code/ (installed via roswell)

There may be a better function to decide if (current-directory) is part of implementation source directory.

image

@sakurawald
Copy link
Contributor Author

sakurawald commented Nov 30, 2024

I think the form (eval '(logical-pathname-translations "SYS")) should be eval in the target repl, not the host repl (the repl that running the lem editor).

image

@sakurawald
Copy link
Contributor Author

Still i don't know how to send the eval form to the connected running repl, or maybe there is some better solutions to this.

Copy link
Member

@cxxxr cxxxr left a comment

Choose a reason for hiding this comment

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

Thank you for the PR.
This suggestion for improvement is wonderful.
I am also grateful that you actually made the suggestion into a PR.

Also, I have some comments and suggestions regarding the changes.
Thank you.

suggestion 1

What about separating the files?

I think it can be separated into a single function: make this particular directory read-only.

Then it seems to me that it would be better to separate each file/package.

;; src/ext/read-only-sources.lisp
(uiop:define-package :lem/read-only-sources
  (:use :cl :lem)
  ...)
(in-package :lem/read-only-sources)

...

(add-hook *switch-to-buffer-hook* 'on-switch-to-buffer)

suggestion 2

How about separating the definition of the rules for making it read-only from the hooks?

For example, the following macro defines a rule.

;; lisp-mode.lisp
(define-read-only-source sbcl-source (directory)
  (dolist (pattern (connection-system-file-patterns (current-connection)))
    (when (pathname-match-p directory pattern)
      (return t))))

The definition of this macro is placed in the same place as the hook.

;; src/ext/read-only-sources.lisp

(defvar *patterns* (make-hash-table :test 'equal))

(defun register-pattern (name function)
  (setf (gethash name *patterns*) function))

(defmacro define-read-only-source (name (directory) &body body)
  `(register-pattern ',name (lambda (,directory) ,@body)))

(defun read-only-source-p (directory)
  (maphash (lambda (name pattern)
             (declare (ignore name))
             (when (funcall pattern directory)
               (return-from read-only-source-p t)))
           *patterns*))

(defun on-switch-to-buffer (buffer)
  (when (and (variable-value 'read-only-sources :default buffer)
             (read-only-source-p (buffer-directory buffer)))
    (setf (buffer-read-only-p buffer) t)))

(add-hook *switch-to-buffer-hook* 'on-switch-to-buffer)

src/lem.lisp Show resolved Hide resolved
src/lem.lisp Show resolved Hide resolved
src/window/window.lisp Show resolved Hide resolved
@sakurawald
Copy link
Contributor Author

sakurawald commented Nov 30, 2024

The Suggestion 1 would be good, to extract the feature into an extension, reducing the complexity of the core source.

The Suggestion 2 makes the read-only a facility to support other purpose, looks nice.

You can close this issue, if you have implement the source in your side.

@cxxxr
Copy link
Member

cxxxr commented Nov 30, 2024

You can close this issue, if you have implement the source in your side.

OK.
I have created the following PR
#1658

Please let me know if there are any deficiencies.

@sakurawald sakurawald closed this Nov 30, 2024
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.

2 participants