Tempo compatibility

[DevelopmentCool2449]

Part of #188, emacs-devel thread and #191 discussions.

Continuation of #191.

I've tested the tempel compatibility in org-tempo.el and it works (even for the tempel.el commands), however it fails because it quotes the elements:

(tempel-tempo-define-template "org-include"
                              '((org-tempo--include-file)
                                p '>)
                                  ^^
                              "<I"
                              "Include keyword"
                              'org-tempo-tags)

This can be fixed by removing these quotes, but it seems that tempo supports this type of syntax. I wonder if tempel should also support this syntax (and warn that this is deprecated).

[minad]

This can be fixed by removing these quotes, but it seems that tempo supports this type of syntax. I wonder if tempel should also support this syntax (and warn that this is deprecated).

Oh I wasn't aware that this syntax is supported. Is it documented? I don't think we should support it.

[DevelopmentCool2449]

This can be fixed by removing these quotes, but it seems that tempo supports this type of syntax. I wonder if tempel should also support this syntax (and warn that this is deprecated).

Oh I wasn't aware that this syntax is supported. Is it documented?

No, It looks like some kind of hack because tempo will eval the element if it is not recognized.

I don't think we should support it.

I agree.

[DevelopmentCool2449]

I've removed some functions and variables that are better have them in tempo.el instead of tempel-tempo.el.

I also realized that tempo.el supports elements returned by functions:
e.g. in snmp-mode.el:

(tempo-define-template "snmp-object-type"
  '(> (P "Object Label: ") " OBJECT-TYPE" n>
    ...
    (P "Default Value: " defval t)
    (if (string= "" (tempo-lookup-named 'defval))
        nil
      '(l "DEFVAL { " (s defval) " }" n>))
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      ...

Should tempel support this or only suport them in tempel-tempo.el?

Update:
Also, I noticed that the tempel commands (tempel-complete and tempel-expand) do not work with org-tempo.el templates, because the templates in org-tempo are prefixed with <, and these commands cannot detect them.
(The (old) tempel-tempo.el commands didn't have this issue.)

This is caused by bounds-of-thing-at-point:

tempel/tempel.el

Line 721 in 1432664

(bounds (bounds-of-thing-at-point 'symbol))

Update 2#:
I've added to tempel the variable: tempel--bound-names, this is intended to be used in tempel-tempo-save-named which is used to bind variables locally in the template (works like (p (any value to bind) name :no-insert, but intended to be used in lisp code inside the template), probably tempel-tempo-save-named and tempel-tempo-lookup-named should be added to tempel.el instead, WDYT.

[minad]

I also realized that tempo.el supports elements returned by functions: e.g. in snmp-mode.el:

(tempo-define-template "snmp-object-type"
  '(> (P "Object Label: ") " OBJECT-TYPE" n>
    ...
    (P "Default Value: " defval t)
    (if (string= "" (tempo-lookup-named 'defval))
        nil
      '(l "DEFVAL { " (s defval) " }" n>))
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      ...

Tempel reevaluates arbitrary forms from the modification hook. Maybe we could try to evaluate first and if the returned value is a listp then treat the return value as fields and expand? What do you think? Instead of (tempo-lookup-named 'defval) one can simply use defval in Tempel.

Should tempel support this or only suport them in tempel-tempo.el?

Yes.

Update: Also, I noticed that the tempel commands (tempel-complete and tempel-expand) do not work with org-tempo.el templates, because the templates in org-tempo are prefixed with <, and these commands cannot detect them. (The (old) tempel-tempo.el commands didn't have this issue.)

This is caused by bounds-of-thing-at-point:

I think we could generalize Tempel and support alternative matchers in addition to bounds-of-thing-at-point?

Update 2#: I've added to tempel the variable: tempel--bound-names, this is intended to be used in tempel-tempo-save-named which is used to bind variables locally in the template (works like (p (any value to bind) name :no-insert, but intended to be used in lisp code inside the template), probably tempel-tempo-save-named and tempel-tempo-lookup-named should be added to tempel.el instead, WDYT.

See my comment #192 (comment). tempel-lookup-named is unnecessary, since one can access the variable directly. Also tempel-save-named should not be necessary, but we could add it if someone needs it. So far no Tempel user has requested it.

[DevelopmentCool2449]

I also realized that tempo.el supports elements returned by functions: e.g. in snmp-mode.el:

(tempo-define-template "snmp-object-type"
  '(> (P "Object Label: ") " OBJECT-TYPE" n>
    ...
    (P "Default Value: " defval t)
    (if (string= "" (tempo-lookup-named 'defval))
        nil
      '(l "DEFVAL { " (s defval) " }" n>))
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      ...

Tempel reevaluates arbitrary forms from the modification hook. Maybe we could try to evaluate first and if the returned value is a listp then treat the return value as fields and expand? What do you think?

I like the idea.

I've implemented something like this, (this duplicates code from tempel--form), WDYT:

tempel/tempel.el

Lines 395 to 399 in 87a174a

	(_ (if-let* ((ret (or (tempel--recursive-list elt)
	(tempel--user-element elt))))
	(tempel--element region ret)
	;; TEMPEL EXTENSION: Evaluate forms
	(tempel--form elt)))))

Instead of (tempo-lookup-named 'defval) one can simply use defval in Tempel.

I agree.

Update: Also, I noticed that the tempel commands (tempel-complete and tempel-expand) do not work with org-tempo.el templates, because the templates in org-tempo are prefixed with <, and these commands cannot detect them. (The (old) tempel-tempo.el commands didn't have this issue.)
This is caused by bounds-of-thing-at-point:

I think we could generalize Tempel and support alternative matchers in addition to bounds-of-thing-at-point?

Yes, In fact, this is what tempo.el does with tempo-match-finder, which is used to identify the name to expand.

Another alternative is to use looking-back:

(looking-back
 (regexp-opt
  (mapcar (lambda (name) (symbol-name (car name)))
          (tempel--templates)))
 (line-beginning-position))

But i'm not sure if this can work.

Update 2#: I've added to tempel the variable: tempel--bound-names, this is intended to be used in tempel-tempo-save-named which is used to bind variables locally in the template (works like (p (any value to bind) name :no-insert, but intended to be used in lisp code inside the template), probably tempel-tempo-save-named and tempel-tempo-lookup-named should be added to tempel.el instead, WDYT.

See my comment #192 (comment). tempel-lookup-named is unnecessary, since one can access the variable directly. Also tempel-save-named should not be necessary, but we could add it if someone needs it. So far no Tempel user has requested it.

Fine, Then I will keep tempel-save-named only in tempel-tempo.el

[minad]

My alternative is to use this functions instead:

Looks good. I pushed your commit with minor modification, see 46d4f54. Does this work well in your tests?

[DevelopmentCool2449]

My alternative is to use this functions instead:

Looks good. I pushed your commit with minor modification, see 46d4f54. Does this work well in your tests?

Yes, it does with tempel-tempo-tests.el and tempo-tests.el, Thank you, I just need to clean up the code.

I found a little problem when expanding lisp code, since they can return string, this code will prompt again:

(tempo-define-template "snmp-object-type"
  '(> (P "Object Label: ") " OBJECT-TYPE" n>
    "SYNTAX  "
    (if tempo-interactive
        (snmp-completing-read "Syntax: " snmp-mode-syntax-list nil nil)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      p) n>
    "ACCESS  "
    ...

However, this is a minor problem, since it can be solved:

(tempo-define-template "snmp-object-type"
  '(> (P "Object Label: ") " OBJECT-TYPE" n>
    "SYNTAX  "
    (if tempo-interactive
        (progn (insert (snmp-completing-read "Syntax: " snmp-mode-syntax-list nil nil))
               nil)
      p) n>
    "ACCESS  "
    ...

Also I wonder if there is the possibility of adding an option to disable the highlighting, since tempo users can probably find it annoying.

[minad]

I found a little problem when expanding lisp code, since they can return string, this code will prompt again:

I think one could write (list (snmp-completing-read "Syntax: " snmp-mode-syntax-list nil nil)) instead. Also Tempel supports (p (snmp-completing-read "Syntax: " snmp-mode-syntax-list nil nil)) as an extension.

[minad]

I would like to add tempo-define-template to Tempel instead. Tempel already has tempel-key and I think tempel-define-template can supersede it. We can use cl-defun with keyword arguments. Then tempel-tempo-define-template would only be a thin wrapper.

[minad]

One problem is that the template management is a bit different in Tempel and Tempo. In Tempel everything is provided dynamically by the template sources, and I've designed Tempel to work well with completion-at-point/completing-read. In contrast, Tempo uses various association lists of template names and functions. We have to find ways to reconcile the two approaches.

[DevelopmentCool2449]

I would like to add tempo-define-template to Tempel instead. Tempel already has tempel-key and I think tempel-define-template can supersede it. We can use cl-defun with keyword arguments. Then tempel-tempo-define-template would only be a thin wrapper.

I agree, but for global and local templates?

tempo-define-template also creates a command to insert the template (probably useful for auto-insert) and a variable, should this be included too?

[DevelopmentCool2449]

One problem is that the template management is a bit different in Tempel and Tempo. In Tempel everything is provided dynamically by the template sources, and I've designed Tempel to work well with completion-at-point/completing-read. In contrast, Tempo uses various association lists of template names and functions. We have to find ways to reconcile the two approaches.

I think we should move all the code from tempel-tempo.el to tempo.el instead.

So tempo users will actually be using Tempel in the background without compromising Tempel code, they can use their tempo commands and option with the same behavior, they even can use the Tempel commands, it should also work as demonstrated in my tests (although slightly different)

[DevelopmentCool2449]

I've added tempel-define-template function, currently it only works for global templates, since i think tempel-path-templates can work for local templates:

Here is a little comparation between tempel-define-template and tempo-define-template:

(tempel-define-template
 :fnname "cmake-library"
 :name "lib"
 :elements
 '((P "project: " proj 'noinsert)
   "cmake_minimum_required(VERSION 3.11)" n n
   "set(CMAKE_POSITION_INDEPENDENT_CODE ON)" n
   "set(CMAKE_EXPORT_COMPILE_COMMANDS ON)" n n
   "project(" (s proj) n
   "  VERSION 0.1.0" n
   "  LANGUAGES CXX" n
   "  DESCRIPTION \"\"" n
   "  HOMEPAGE_URL \"\"" n
   ")" n n
   "include(CTest)" n
   "include(FetchContent)" n n)
 :doc "Insert a cmake library")

(tempo-define-template
 "cmake-library" ; fnname
 '((P "project: " proj 'noinsert)  ; elements
   "cmake_minimum_required(VERSION 3.11)" n n
   "set(CMAKE_POSITION_INDEPENDENT_CODE ON)" n
   "set(CMAKE_EXPORT_COMPILE_COMMANDS ON)" n n
   "project(" (s proj) n
   "  VERSION 0.1.0" n
   "  LANGUAGES CXX" n
   "  DESCRIPTION \"\"" n
   "  HOMEPAGE_URL \"\"" n
   ")" n n
   "include(CTest)" n
   "include(FetchContent)" n n)
 "lib" ; name to expand
 "Insert a cmake library" ; doc
 ;; 'taglist (probably useless in `tempel-define-template')
 )

tempel-define-template works:

[minad]

I thought a while about this, and I am not anymore sure that we should add tempel-define-template. In Tempel we can easily define additional templates without the need of such a function, see
https://github.com/minad/tempel?tab=readme-ov-file#adding-templates. Modes can also take advantage of this and create their own template lists. Then the mode can add the list locally to tempel-template-sources.

[DevelopmentCool2449]

Fine, I've removed tempel-define-template.

[minad]

Regarding force pushing - I suggest to avoid this. Then it will be easier to follow the progress, and I could also apply some improvements to the code. We can still cleanup everything in the end, and the tempel-tempo adaptor will rather go to Emacs anyway?

[DevelopmentCool2449]

Regarding force pushing - I suggest to avoid this. Then it will be easier to follow the progress, and I could also apply some improvements to the code.

I'm sorry, i used the force-push to keep the commit history clearer.

We can still cleanup everything in the end, and the tempel-tempo adaptor will rather go to Emacs anyway?

Yes, although I already have tempo.el rewritten using all the code from tempel-tempo.el.

I think if there are no more suggestions, this PR could be merged.

I can announce this in the emacs-devel thread and see if there's anything else to add/change in tempel.

[minad]

I'm sorry, i used the force-push to keep the commit history clearer.

Yes, sure. I know. Could you please push the compatibility code here again - the current state? I would like to add some improvements as well if you agree.

Yes, although I already have tempo.el rewritten using all the code from tempel-tempo.el.

I mean this code. You can add your tempo.el here.

I can announce this in the emacs-devel thread and see if there's anything else to add/change in tempel.

I thought we could try two approaches. Either replace tempo.el completely with the adapter, or provide an additional variant of tempo.el based on Tempel, which would be loaded if some customization is set.

[minad]

Regarding only-prompt - I am not sure if I had already commented on that. (The inline comment history seems also affected by force pushes). In Tempel every field is "interactive". In contrast, reading from the minibuffer affects the experience negatively since template expansion is paused until the result from the minibuffer has been read. Therefore I had not supported this feature in Tempel until now. read-string is only used if noinsert is specified, since then there is no other possibility to read the input.

[DevelopmentCool2449]

Regarding only-prompt - I am not sure if I had already commented on that. (The inline comment history seems also affected by force pushes).

Oh, i didn't know it, I'm sorry.

In Tempel every field is "interactive". In contrast, reading from the minibuffer affects the experience negatively since template expansion is paused until the result from the minibuffer has been read. Therefore I had not supported this feature in Tempel until now. read-string is only used if noinsert is specified, since then there is no other possibility to read the input.

Fine, i've removed this part.

Although this will break some tests in tempel-tempo-tests.el, I'll see how to fix this.

[minad]

We can maybe keep the prompt argument such that you can override it in tempo.el. However I would not change the behavior in Tempel itself. But then, if we use Tempel as backend anyway, then we can simply consistenly use Tempel behavior. This means simply removing support for tempo-interactive.

[DevelopmentCool2449]

I'm sorry, i used the force-push to keep the commit history clearer.

Yes, sure. I know. Could you please push the compatibility code here again - the current state? I would like to add some improvements as well if you agree.

I've restored tempel-tempo.el and tempel-tempo-tests.el

Yes, although I already have tempo.el rewritten using all the code from tempel-tempo.el.

I mean this code. You can add your tempo.el here.

Ok, i've added it.

I can announce this in the emacs-devel thread and see if there's anything else to add/change in tempel.

I thought we could try two approaches. Either replace tempo.el completely with the adapter, or provide an additional variant of tempo.el based on Tempel, which would be loaded if some customization is set.

Sounds good to me.

[minad]

You've added tempel-tempo.el and tempo.el here. How do they relate? Do you plan to move all code to tempo.el? The tempo.el still contains a lot of code which is obsolete, like tempo-show-completion-buffer and so on.

[DevelopmentCool2449]

[ I apologize for the delay, I've been very busy (and probably still will be) and haven't had the time to reply ]

You've added tempel-tempo.el and tempo.el here. How do they relate?

tempo.el includes all the code from tempel-tempo.el, making the tempo commands and defined templates (by tempo-define-template) 100% (mostly) compatible with tempel templates/commands and vice versa.

tempel-tempo.el only includes some functions from tempo such as tempel-tempo-define-template.

Do you plan to move all code to tempo.el? The tempo.el still contains a lot of code which is obsolete, like tempo-show-completion-buffer and so on.

I think only the useful and core tempo functions (not commands) should be moved to tempel-tempo.el, and leaving the obsolete things in tempo.el. WDYT

[minad]

I've also been busy with some other things. My suggestion would be this:

1. Create a simple tempo.el file with this content:

;;; tempo.el ---  -*- lexical-binding: t -*-
;;; Commentary:
;;; Code:

(defcustom tempo-use-tempel t
  "Use Tempel."
  :type 'boolean)

;; ... shared definitions ...

(require (if tempo-use-tempel 'tempo-tempel 'tempo-obsolete))

(provide 'tempo.el)
;;; tempo.el ends here

2. Create a tempo-tempel.el file with Tempel specific code.
3. Retain all the remaining old code in tempo-obsolete.el

This way we can remove unwanted features more freely from tempo-tempel. We would still be mostly compatible. We keep tempo-obsolete in case someone wants 100% backward compatibility. Furthermore we can compare the two tempo profiles side by side. All this should only involve copying definitions around, and use what you already have. What do you think? (Please preserve the commit history here, if you can.)

[DevelopmentCool2449]

Sorry for the delay, I've been very busy, but i think I'm back in action.

My suggestion would be this:

1. Create a simple tempo.el file with this content:

;;; tempo.el ---  -*- lexical-binding: t -*-
;;; Commentary:
;;; Code:

(defcustom tempo-use-tempel t
  "Use Tempel."
  :type 'boolean)

;; ... shared definitions ...

(require (if tempo-use-tempel 'tempo-tempel 'tempo-obsolete))

(provide 'tempo.el)
;;; tempo.el ends here

2. Create a tempo-tempel.el file with Tempel specific code.
3. Retain all the remaining old code in tempo-obsolete.el

This way we can remove unwanted features more freely from tempo-tempel. We would still be mostly compatible. We keep tempo-obsolete in case someone wants 100% backward compatibility. Furthermore we can compare the two tempo profiles side by side. All this should only involve copying definitions around, and use what you already have. What do you think?

I like the idea, probably this or next week i will have this done.

[DevelopmentCool2449]

Sorry for the noise (and disappointment), but I'm not sure how to proceed with this, I have some ideas on how to finish it, but I still don't know which one to choose. Also, this feature was meant to allow to merge Tempel into core. Currently, I think Tempel is perfectly fine where it is (in NonGNU ELPA), so I'm not sure if you share this opinion.

What I could do is add some modern Tempo features. For example, I think a tempel-define-template counterpart would be useful, since I have templates that I prefer to have written in my init.el file (also to use them with autoinsert). Perhaps other packages might find this useful and gradually replace Tempo in the long run.

What do you think?

[minad]

Also, this feature was meant to allow to merge Tempel into core.

Yes, this was the idea. My comment shows a way to do this: #192 (comment)

The idea was that Tempo/Tempel are prepared here in this PR, but just as a staging ground for us to discuss. It won't actually be merged here. Some changes to Tempel have already been merged which improved compatibility, and more could be needed.

What I could do is add some modern Tempo features. For example, I think a tempel-define-template counterpart would be useful, since I have templates that I prefer to have written in my init.el file (also to use them with autoinsert).

As I see it, there are no "modern" Tempo features. When I wrote Tempel I extracted all relevant parts and modernized the package as a whole. Instead of tempel-define-template one can use a variable as described in https://github.com/minad/tempel?tab=readme-ov-file#adding-templates. This requires less boilerplate and is more lispy. You simply use the Lisp language elements and Sexps without additional macros, which is more obvious for beginners.