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

Optional named field argument for custom user element hooks #153

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

okomestudio
Copy link

This PR

  • updates the API for custom user element hook to accept an (optional) second argument `fields', an alist of field name-value pairs
  • adds a section in README describing the usage of the new API

See #152 for the wider context.

@okomestudio okomestudio changed the title Custom element function with st Optional named field argument for custom user element hooks Oct 28, 2024
@minad
Copy link
Owner

minad commented Oct 29, 2024

Thanks. Can you please provide a simple example in the README which is not artificial, something one would actually want to use, like tempel-include?

@okomestudio
Copy link
Author

Thanks. Can you please provide a simple example in the README which is not artificial, something one would actually want to use, like tempel-include?

I've added an example. I wish that I have a shorter example better suited for README, but I cannot come up with a clever idea right now. The example is close to my current use case. If you have another suggestion, I'd update accordingly.

@minad
Copy link
Owner

minad commented Oct 30, 2024

Hmm, I wonder about this example. I think the right way to implement this would be to read the file name via read-from-minibuffer or read-file-name in the custom element, such that you would get completion on the available files. Then we wouldn't need this addition.

Can we think of something more basic? Something which uses the variables in some more direct way? Maybe some examples which performs some computations? On the other hand we can already embed arbitrary Lisp FORMS in Tempel. Ideally I would want an example which people would want to use directly, similar to the tempel-include example - I am using this in my config directly as is. I have this in my set of templates for example:

(header ";;; " (file-name-nondirectory (or (buffer-file-name) (buffer-name)))
        " --- " p " -*- lexical-binding: t -*-" n
        ";;; Commentary:" n ";;; Code:" n n)
(provide "(provide '" (file-name-base (or (buffer-file-name) (buffer-name))) ")" n
         ";;; " (file-name-nondirectory (or (buffer-file-name) (buffer-name)))
         " ends here" n)
(package (i header) r n n (i provide)) ;; uses my tempel-include user element

To be clear, I think every addition should be justified and if we cannot come up with a reasonable example, I think the addition is not justified and we should maybe not add it. Note that every addition limits our future options and increases complexity.

@okomestudio
Copy link
Author

To be clear, I think every addition should be justified and if we cannot come up with a reasonable example, I think the addition is not justified and we should maybe not add it. Note that every addition limits our future options and increases complexity.

I fully agree with keeping the API clean. It's not like I am strongly advocating for this to get merged, and I would've used another option if my use case was covered by it. Let me explain what led me here.

Hmm, I wonder about this example. I think the right way to implement this would be to read the file name via read-from-minibuffer or read-file-name in the custom element. Then we wouldn't need this addition.

My main goal was to load a source code template from a local file, which may be shared with tools outside Emacs. So I cannot simply migrate it and have it as a tempel template. And I didn't necessarily want to set the file name interactively, as the template files are stored in a vault and wanted to cover the use case in which the file name is inferred from something else (e.g., the tempel template name suffixed with elisp inferring that I want to load template for Elisp code, so hard coding the file name is more desirable than prompting for it).

Being a template, variable expansion is obviously useful, so I looked for a way to perform named field expansion on an arbitrary string. It didn't seem it's supported by an existing feature, so I turned to the custom user element with which an "arbitrary" hook can be written with flexibility.

If I were to strongly advocate for custom user element hooks to (optionally) take in (cdr st), I'd say the reason would be the feature parity and symmetry with the "form"-style element. In some sense, with the "form" extension of tempel, we should be able to do quite a bit of things using the feature. Then the custom user element hook may be considered or used to wrap that form, for code reuse. Except that it's not straightforward to do so, because the "form" extension runs it giving the context by way of (eval form (cdr st)), but custom user element hooks don't have access to (cdr st) at all. This seems a reasonable ask, and perhaps the hook API should've considered this feature alignment to begin with; I understand that the backward compatibility precluded this option, though.

(Writing the last paragraph, I realized another option might be to write a regular elisp function, and use the "form" extension to achieve similar goals; but then, the explicit access to (cdr st) might need some trick to make happen...)

With all that said, I do wish there was a more compleling, simpler use case to show case in README that strongly justifies the usefulness of named fields being available inside the hooks.

In any case, this is where I came from. Thanks for your patience.

@minad
Copy link
Owner

minad commented Oct 30, 2024

I mean it makes sense to have context access in the custom elements, like we have in form elements. But for forms it is completely obvious that we need it, since the most trivial computations based on other fields require it. This means an Elisp function called in a form works too in many cases. User elements are also a little bit of a different beast, since they return a new template list, which is interpreted again, and not a string. The addition is simple enough, so we could as well merge it. I just don't like the idea of not having a simple example to show, which means that we miss some understanding. Maybe the idea of user elements is wrong and unnecessary in the first place since one could maybe always use forms, except for the verbosity (user elements may be a bit of a legacy concept inherited from tempo)? In any case, it seems there is no urgency to get this merged, so we can as well let this rest for a while until we come up with something.

@okomestudio
Copy link
Author

... In any case, it seems there is no urgency to get this merged, so we can as well let this rest for a while until we come up with something.

Sounds good to me. I'll come back and add a comment if I come up with a simpler demo. (Someone else might also chime in with another use case.)

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