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

Functional config & hooks #419

Closed
Ambrevar opened this issue Sep 28, 2019 · 8 comments
Closed

Functional config & hooks #419

Ambrevar opened this issue Sep 28, 2019 · 8 comments

Comments

@Ambrevar
Copy link
Member

Ambrevar commented Sep 28, 2019

A discussion on the general idea of configurability: This is an iteration on
the question of hooks (#383) and
the controversial topic of functional configuration
(#397).


I just had an epiphany when working with the set-url hook:
the handlers take the URL as argument, but not the buffer. So how do the
handler know which buffer they apply to. Calling (current-buffer) is wrong:
for instance, when we call reload-buffer over multiple buffers,
(current-buffer) always returns the same buffer, not the buffer being
reloaded.

Possible fixes:

  • Add a buffer parameter. But it fits badly with run-composed-hook which
    calls the next handler over the return value of the previous. Even if we pass
    the buffer along, we've got no strong guarantee that no handler is returning a
    different buffer, thus breaking the integrity of the hook.
    Beside, it really feels like we are passing along an information that should
    already be there, because the hook belongs to the buffer.

  • Replace the named handler by a closure over the buffer. This seems right, but
    there is a problem: a closure is anonymous, and hooks don't like lambdas as
    the Emacs experience tells us:

    • Lambdas are not comparable and thus duplicates can't be removed and will
      stack up.

    • They are black boxes to the user, we can't know what they do.

    • It's impractical to remove a lambda interactively since it has no name.

So what if we created a handler class with the following slots:

  • Name / description, e.g. handler that sets the search engine to '("foo" "bar").
  • The function, possibly a lambda.
  • place: (Optional) If the function is a setter, describe the place being set,
    e.g. '(remote-interface buffer-make-hook load-hook).
  • value: (Optional) If the function is a setter, describe the value,
    e.g. '(1 2 3).

We would modify add-to-hook to either accept functions or handlers but not
anonymous functions.
Then run-hooks should be able to run both regular functions and
the above handlers in a hook.

Handlers should be comparable so that the hooks can remove duplicates.
I can think of various criteria for comparison:

  • If function is the same (non-lambda only).
  • Or, if place and value are set, if they are EQUAL. This is only useful for
    setters.
  • I'm not sure it makes sense to compare the name/description.

In #397, we discussed new
"functional" (stateless) ways to configure Next.
I was suggesting

(defun my-minibuffer-defaults (minibuffer)
  (setf
   (minibuffer-line-height minibuffer) "1.1em")

(defun my-interface-defaults ()
  (hooks:add-to-hook (hooks:object-hook *interface* 'minibuffer-make-hook)
                     #'my-minibuffer-defaults))

(hooks:add-to-hook '*after-init-hook* #'my-interface-defaults)

Obviously this is not the easiest.

We could use helper macros but they would necessarily add unnamed handlers to
hooks... Which is not a problem anymore if we use the above handler types!

Then we can write macros to set default values (dummy code):

(defmacro set-default (object slot value)
  ...
  (cond
   ((eq object 'buffer)
    (add-hook *after-init-hook*
        (make-handler
           :description (format nil "Register handler to set buffer's ~a to ~a" slot value)
           :place '(remote-interface buffer-make-hook)
           :value value
           :function (lambda ()
                      (add-hook buffer
                         (make-handler
                           :description (format nil "Set buffer's ~a to ~a" slot value)
                           :place slot
                           :value value
                           :function (lambda (buffer)
                                       (setf (slot-value buffer slot) value))))))))
   ...))

This would allow us to set values this way:

(set-default 'buffer 'minibuffer-line-height "1.1em")

We could also support

(set-default 'buffer
  'minibuffer-line-height "1.1em"
  'minibuffer-font-size "1.0em")

to avoid flooding *after-init-hook*.

Thoughts?

@Ambrevar
Copy link
Member Author

@vindarel's suggestion: Use (set-buffer-default 'minibuffer-line-height "1.1em"), this way we have completion of the different classes for the "set default" action.

@vindarel
Copy link
Contributor

yeah +1 as discussed I find it a small step but a grand usability and code stability improvement.

@Ambrevar
Copy link
Member Author

Ambrevar commented Nov 6, 2019

Maybe a (much?) better idea: If we look at the big picture, what we are trying to do here is specialize a class (in the general sense).

So what if we did the following: In Next source, define

(declaim (type (remote-interface) remote-interface-class))
(defparameter remote-interface-class (find-class 'remote-interface))

; ...

; Then replace every `make-instance 'remote-interface` with
(make-instance (class-name remote-interface-class) ...)

Now from the user init.lisp:

(defclass my-interface (remote-interface)
  ((search-engines :initarg my-newvalue-here)
	 (other-slot :initarg my-other-default-here)))

(setf remote-interface-class (find-class 'my-interface))

Note that remote-interface-class is a class, this way we can enfore type-checking and only allow subclasses of remote-interface.

This would be illegal (and caught at compile-time):

(setf remote-interface-class (find-class 'buffer))

So this allows third-party packages to offer defaults to the user.

A key feature here is that CLOS supports multiple inheritance: this effectively allows us to compose defaults.

Say, some contributors develop Doom-Next and some others Space-Next.
As a user, I may want some of the configs from Doom-Next, others from Space-Next.
If I want to prioritize Doom-Next over Space-Next, I would do:

(defclass my-interface (doom-next:remote-interface space-next:remote)
  ())

(setf remote-interface-class (find-class 'my-interface))

Or, the other way around, I would change the order of the superclasses:

(defclass my-interface (space-next:remote doom-next:remote-interface)
  ())

(setf remote-interface-class (find-class 'my-interface))

We would always have access to all default values of all classes. The type of remote-interface-class tells us which is the default class.


Note that it does not remove the need for hooks as mentioned above.
It only removes the need for hooks just to configure default values of slots.

Thoughts?
@jmercouris @vindarel

@Ambrevar
Copy link
Member Author

Ambrevar commented Nov 6, 2019

The above example was for remote-interface: we would obviously do the same for all classes (window, buffer, etc.).

@Ambrevar
Copy link
Member Author

Ambrevar commented Nov 6, 2019

Forgot one nicety: the remote-interface-class can be let-bound, so that specialization happens only locally, e.g.

(let ((buffer-class (find-class 'my-other-class)))
  (make-buffer...))

; Back to normal here.

@Ambrevar
Copy link
Member Author

Ambrevar commented Nov 6, 2019

A gotcha: slot names could be mistyped. For instance if the user writes

(defclass my-interface (remote-interface)
  ((seach-engines :initarg my-newvalue-here)
	 (other-slot :initarg my-other-default-here)))

(setf remote-interface-class (find-class 'my-interface))

they might be puzzled as for why the search-engines settings have not been applied. (Note the missing r.)

One way around this: use initialize-instance :after. Example:

(defclass my-interface (remote-interface)
  ())  

(defmethod initialize-instance :after ((interface my-interface))
  (setf (search-engines interface) my-new-value-here))

@Ambrevar
Copy link
Member Author

Ambrevar commented Nov 7, 2019

See #484 for an implementation.

@Ambrevar
Copy link
Member Author

Closing since #484 has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants