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

consult-yasnippet #173

Closed
wants to merge 3 commits into from
Closed

consult-yasnippet #173

wants to merge 3 commits into from

Conversation

mohkale
Copy link
Contributor

@mohkale mohkale commented Jan 20, 2021

This command works as a parallel to ivy-yasnippet and allows you to
expand yasnippet snippets through a completing-read interface. It
supports previewing the current snippet expansion and overwriting the
marked region with a new snippet completion.

Embark

This package also provides the consult-yasnippet-visit-snippet-file
package. Which can be used as an alternative action for
consult-yasnippet through for eg. embark:

(require 'embark)

(embark-define-keymap embark-yasnippet-completion-actions
                      "Embark actions for `consult-yasnippet' and derivatives"
                      ;; NOTE: Binding differs from `ivy-yasnippet' which uses "v".
                      ("d" consult-yasnippet-visit-snippet-file))

(add-to-list 'embark-keymap-alist '(yasnippet . embark-yasnippet-completion-actions))

marginalia

The default completion interface simply shows the name associated with a
snippet. You can annotate it with the prefix/key needed to expand to it
using marginalia.

(require 'marginalia)

(defun marginalia-annotate-yasnippet (cand)
  ;; NOTE: Maybe there's a less indirect way to do this :/.
  (when-let* ((cand (marginalia--full-candidate cand))
              (template (alist-get cand consult-yasnippet--snippets nil nil #'string-equal))
              (key (yas--template-key template)))
    (concat " " (propertize (concat "[" key "]")
                            ;; TODO: custom face
                            'face 'font-lock-type-face))))

(push '(yasnippet . marginalia-annotate-yasnippet) marginalia-annotators-light)
(push '(yasnippet . marginalia-annotate-yasnippet) marginalia-annotators-heavy)

This command works as a parallel to [[https://github.com/mkcms/ivy-yasnippet][ivy-yasnippet]] and allows you to
expand yasnippet snippets through a completing-read interface. It
supports previewing the current snippet expansion and overwriting the
marked region with a new snippet completion.

** Embark
This package also provides the `consult-yasnippet-visit-snippet-file`
package. Which can be used as an alternative action for
`consult-yasnippet` through for eg. embark:

```lisp
(require 'embark)

(embark-define-keymap embark-yasnippet-completion-actions
                      "Embark actions for `consult-yasnippet' and derivatives"
                      ;; NOTE: Binding differs from `ivy-yasnippet' which uses "v".
                      ("d" consult-yasnippet-visit-snippet-file))

(add-to-list 'embark-keymap-alist '(yasnippet . embark-yasnippet-completion-actions))
```

** marginalia
The default completion interface simply shows the name associated with a
snippet. You can annotate it with the prefix/key needed to expand to it
using marginalia.

```lisp
(require 'marginalia)

(defun marginalia-annotate-yasnippet (cand)
  ;; NOTE: Maybe there's a less indirect way to do this :/.
  (when-let* ((cand (marginalia--full-candidate cand))
              (template (alist-get cand consult-yasnippet--snippets nil nil #'string-equal))
              (key (yas--template-key template)))
    (concat " " (propertize (concat "[" key "]")
                            ;; TODO: custom face
                            'face 'font-lock-type-face))))

(push '(yasnippet . marginalia-annotate-yasnippet) marginalia-annotators-light)
(push '(yasnippet . marginalia-annotate-yasnippet) marginalia-annotators-heavy)
```
@mohkale
Copy link
Contributor Author

mohkale commented Jan 20, 2021

@minad

Please let me know if there's anything else you'd like me to add/modify.

@minad
Copy link
Owner

minad commented Jan 20, 2021

Thanks, Mohsin! I will give you a review in the source. It is a good idea to provide Embark action commands here. But instead of Marginalia it is better to annotate the candidates directly using the 'display property.

(propertize "prefix" 'display "prefix-annotation")

README.org Outdated Show resolved Hide resolved
consult-yasnippet.el Outdated Show resolved Hide resolved
consult-yasnippet.el Outdated Show resolved Hide resolved
consult-yasnippet.el Outdated Show resolved Hide resolved
consult-yasnippet.el Outdated Show resolved Hide resolved
consult-yasnippet.el Outdated Show resolved Hide resolved
@minad
Copy link
Owner

minad commented Jan 20, 2021

The main improvements I would like to see is the change to lexically captured state, since this fits better with how things are done in the other Consult functions. Probably that is different from how Ivy/Counsel is handling its state.

See consult--preview-position

consult/consult.el

Lines 848 to 858 in 6d45873

(defun consult--preview-position (&optional face)
"The preview function used if selecting from a list of candidate positions.
The function can be used as the `:preview' argument of `consult--read'.
FACE is the cursor face."
(let ((overlays)
(invisible)
(face (or face 'consult-preview-cursor))
(saved-min (point-min-marker))
(saved-max (point-max-marker))
(saved-pos (point-marker)))
(lambda (cand restore)
or consult--yank-read

consult/consult.el

Lines 2167 to 2169 in 6d45873

;; If previous command is yank, hide previously yanked text
(let* ((ov) (pt (point)) (mk (or (and (eq last-command 'yank) (mark t)) pt)))
(lambda (cand restore)

Furthermore you apply the modification in the preview directly, it seems. This modifies the undo state. So it would be better if we use overlays here to preview the snippet without editing anything as is done in consult--yank-read.

mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
consult-yasnippet.el Outdated Show resolved Hide resolved
consult-yasnippet.el Outdated Show resolved Hide resolved
mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
@mohkale
Copy link
Contributor Author

mohkale commented Jan 20, 2021

@minad I've been meaning to say but I think consult--read needs better documentation messages. I find some of it hard to grok on the face of it. For example :lookup maps a candidate as it was in the original candidate list before passing to :preview or returning from consult--read. The docstring at the moment doesn't really express this. Similarly :preview doesn't describe the form or implied contract of the preview function. The behaviour you've described above would've been really nice to have available in the docs.

@minad
Copy link
Owner

minad commented Jan 20, 2021

Well, it is private API. So that's what you've got for now :)

I agree that documentation should generally be improved. But time is limited and the project is still young. As of now my focus was more on the code and the user facing documentation.

@mohkale
Copy link
Contributor Author

mohkale commented Jan 20, 2021

@minad that's fair. Guess this whole package is so cool I'm kind of really eager to understand how it all works 👍.

@minad
Copy link
Owner

minad commented Jan 20, 2021

I should also add - the internals are still in flux. The behavior of the :preview restore has been changed very recently when I added consult-keep-lines and consult-focus-lines. Before that :preview was only called with activated previews and not always during restore. But since the setup of the closure is executed always it kind of makes more sense to always call the destructor/restoration too. And it made it easier to implement consult-keep-lines and consult-focus-lines :)

@mohkale
Copy link
Contributor Author

mohkale commented Jan 20, 2021

@minad

I've never really used overlays before, but from the looks of it I'll have to apply an overlay to the region with the display property set to an expansion of the snippet. I'm kinda not sure how to get that expansion though :? ivy-yasnippet just expands the entire snippet and then performs substitutions. I could expand it in a temp buffer but I'm not sure whether all the associated font-lock rules would still be in affect. Is this what you're suggesting?

@mohkale
Copy link
Contributor Author

mohkale commented Jan 20, 2021

@minad

So I'm trying out overlays... but yeah, if I expand in a temp-buffer I lose any syntax highlighting in the snippet expansion.

(defun consult-yasnippet--foo (template)
  (with-temp-buffer
    (yas-minor-mode +1)
    (unwind-protect
        (yas-expand-snippet (yas--template-content template)
                            nil nil
                            (yas--template-expand-env template))
      (unwind-protect
          (mapc #'yas--commit-snippet
                (yas-active-snippets (point-min) (point-max)))))
    (buffer-substring (point-min) (point-max))))

Screenshot_20210120_184235

@minad
Copy link
Owner

minad commented Jan 20, 2021

I've never really used overlays before, but from the looks of it I'll have to apply an overlay to the region with the display property set to an expansion of the snippet. I'm kinda not sure how to get that expansion though :? ivy-yasnippet just expands the entire snippet and then performs substitutions. I could expand it in a temp buffer but I'm not sure whether all the associated font-lock rules would still be in affect. Is this what you're suggesting?

Yes, maybe using a separate buffer could be an option. If you use a separate buffer you must enable the correct mode there. But let's think a bit about it. Maybe it is better to perform the expansion in-place during preview

@mohkale
Copy link
Contributor Author

mohkale commented Jan 20, 2021

Maybe it is better to perform the expansion in-place during preview

What do you mean? Save point, expand snippet, delete expansion, show overlay with contents of expansion.

@minad
Copy link
Owner

minad commented Jan 20, 2021

What do you mean? Save point, expand snippet, delete expansion, show overlay with contents of expansion.

Maybe that would be an option. I am not sure about the best way, but I am sure we will figure it out.

@mohkale
Copy link
Contributor Author

mohkale commented Jan 20, 2021

@minad

Okay, the approach I described above does work :-).

(defun consult-yasnippet--foo (template orig-offset)
  "Expand and return contents of TEMPLATE.
ORIG-OFFSET is the distance from "
  (let ((pos (point)))
    (unwind-protect
        (yas-expand-snippet (yas--template-content template)
                            nil nil
                            (yas--template-expand-env template))
      (unwind-protect
          (mapc #'yas--commit-snippet
                (yas-active-snippets (point-min) (point-max)))))
    (let ((end (- (point-max) orig-offset)))
      (prog1 (buffer-substring pos end)
        (delete-region pos end)))))

(defun consult-yasnippet--preview ()
  (let* ((buf (current-buffer))
         (pos (point))
         (region (if (region-active-p)
                     (cons (region-beginning) (region-end))
                   (cons (point) (progn (insert " ") (point)))))
         ;; (region-contents (buffer-substring (car region) (cdr region)))
         (overlays)
         (invisible)
         )
    (lambda (template restore)
      (with-current-buffer buf
        (consult--invisible-restore invisible)
        (mapc #'delete-overlay overlays)
        (cond
         (restore
          (delete-region (car region) (cdr region))
          )
         (template
          (setq invisible (consult--invisible-show))
          (let ((yas-verbosity 0)
                (inhibit-ad-only t)
                (yas-prompt-functions '(yas-no-prompt))
                (orig-offset (- (point-max) (cdr region))))
            (setq overlays (list (consult--overlay
                                  (car region) (cdr region)
                                  'display (consult-yasnippet--foo template orig-offset)
                                  ))))
          )
         )
        ))))

But I'm having issues with geting the overlay to appear. It sounds like overlays need to be applied over at least one character and for some reason when I apply it over multiple character the overlay gets truncated. Honestly I'm not sure when I'll have the time to properly understand how overlays work so hopefully I can come back to this later. For now the writing into the buffer approach works pretty well for me.

@minad
Copy link
Owner

minad commented Jan 20, 2021

Okay, I think I am also fine with modifying the buffer. However you should ensure that the buffer modification status and undo status is kept intact. I just reworked consult-keep-lines, see ef08e03. There is the function with-silent-modifications which may be useful for you too.

I will add some more comments to the current version.

consult-yasnippet.el Outdated Show resolved Hide resolved
@minad
Copy link
Owner

minad commented Jan 20, 2021

Okay, I added a few more comments. Most important is that undo is still working with preview. Please try to address that. Then I will give it another test and maybe merge it as is or apply some more minor adjustments myself.

@minad minad mentioned this pull request Jan 20, 2021
20 tasks
mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
consult-yasnippet.el Outdated Show resolved Hide resolved
consult-yasnippet.el Outdated Show resolved Hide resolved
mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
mohkale added a commit to mohkale/consult that referenced this pull request Jan 20, 2021
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.

None yet

3 participants