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

Make elixir-ts-mode-hook a customizable variable #37

Merged
merged 1 commit into from
Nov 15, 2023

Conversation

sirikid
Copy link
Contributor

@sirikid sirikid commented Nov 13, 2023

No description provided.

@wkirschbaum
Copy link
Owner

@sirikid what problem are you trying to solve here? I have not seen this done on other modes and not sure what it achieves.

@sirikid
Copy link
Contributor Author

sirikid commented Nov 14, 2023

@sirikid what problem are you trying to solve here? I have not seen this done on other modes and not sure what it achieves.

Language mode hooks are intended for user customization thus they should be accessible through Custom. I think most of the built-in hooks are customizable at this point.

@wkirschbaum
Copy link
Owner

@sirikid there is already a elixir-ts-mode hook which is part of deriving from prog-mode. what am I missing here?

You can already do something like this:

(add-hook 'elixir-ts-mode-hook (lambda () (subword-mode)))

@wkirschbaum
Copy link
Owner

I had a look at some other modes and saw lua-ts-mode is the only -ts-mode with this variable. Let me do some reason to see how it works. If you can give me an example it would also be great.

@sirikid
Copy link
Contributor Author

sirikid commented Nov 14, 2023

I had a look at some other modes and saw lua-ts-mode is the only -ts-mode with this variable. Let me do some reason to see how it works. If you can give me an example it would also be great.

Custom is an Emacs subsystem for interactive customization. Here you can see my custom-file, generated by this system, and then a screenshot of the interface.

On the left is the customization of the hook and on the right is the customization of another variable with a relatively complex structure (alist).

image

@wkirschbaum
Copy link
Owner

I am guessing the proposed change allows you to use the customize menu instead of having to do it in your init.el file? Not many modes have this, so just trying to understand. The PR came with no motivation or description.

@sirikid
Copy link
Contributor Author

sirikid commented Nov 14, 2023

I am guessing the proposed change allows you to use the customize menu instead of having to do it in your init.el file?

Yes.

Not many modes have this, so just trying to understand.

I had a different experience.

The PR came with no motivation or description.

Sorry, I thought it was a common knowledge.

@wkirschbaum
Copy link
Owner

wkirschbaum commented Nov 14, 2023

Not many modes have this, so just trying to understand.

I had a different experience.

You can view the emacs source code and will find it is in only a handful of progmodes.

The PR came with no motivation or description.

Sorry, I thought it was a common knowledge.

Even so, its still good to give some motivation.

I can't see a reason not to add it after having a look. Please add an appropriate comment as below then I can merge:

(defcustom c-mode-hook nil
  "Hook called by `c-mode'."
  :type 'hook
  :group 'c)

You don't need the comment either.

@@ -613,6 +613,12 @@ Return nil if NODE is not a defun node or doesn't have a name."
(save-excursion
(newline 1 t))))

(defcustom elixir-ts-mode-hook '()
"Hook run after entering Elixir mode."
Copy link
Owner

Choose a reason for hiding this comment

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

"Hook run after entering `elixir-ts-mode'." its not Elixir mode.

@@ -613,6 +613,12 @@ Return nil if NODE is not a defun node or doesn't have a name."
(save-excursion
(newline 1 t))))

(defcustom elixir-ts-mode-hook '()
Copy link
Owner

Choose a reason for hiding this comment

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

The convention is to specify 'nil' here. Even though its the same it is still good to stick with conventions.

"Hook run after entering Elixir mode."
:group 'elixir-ts
:type 'hook
:options (list #'eglot-ensure))
Copy link
Owner

Choose a reason for hiding this comment

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

Please use the form '(eglot-ensure) as this seems to be the convention.

@wkirschbaum wkirschbaum merged commit 7b52259 into wkirschbaum:main Nov 15, 2023
2 checks passed
@sirikid sirikid deleted the customizable-mode-hook branch November 15, 2023 12:43
This pull request was closed.
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