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

BUG? Transformation upon yas-moving-away-p doesn't update the field #381

Closed
immerrr opened this issue Jun 3, 2013 · 32 comments · Fixed by #689
Closed

BUG? Transformation upon yas-moving-away-p doesn't update the field #381

immerrr opened this issue Jun 3, 2013 · 32 comments · Fixed by #689
Labels

Comments

@immerrr
Copy link
Contributor

immerrr commented Jun 3, 2013

I'm getting weird results trying to do something like

${1:$(when yas-moving-away-p (ensure-is-quoted text))}

Basically, nothing is replaced, but I see ensure-is-quoted is being executed.

It appears that in yas-next-field the transform is evaluated with yas--eval-lisp, but the field is never updated. Is this intentional? The doc -- slightly outdated -- mentions yas/moving-away-p, but doesn't say anything specific about its use.

@joaotavora
Copy link
Owner

Hi,

This is possibly intentional. Unfortunately, I don't know anymore and the documentation is, as you say, outdated.

In general, mirror transforms are designed to return strings that replace the current mirror or field content. yas-moving-away-p was invented for primary field transformations, and mostly for yas-choose-value and yas-verify-value. Its behaviour is probably undefined for mirror transforms.

This is not to discourage you. If you can:

  • Invent a useful and consistent meaning for the variable in mirror transforms
  • implement field updating
  • design tests for that use case (positive and negative case)
  • have all existing tests pass

Then I will probably merge it. I'm changing the title of this issue and marking it a feature request.

@immerrr
Copy link
Contributor Author

immerrr commented Jun 4, 2013

Thanks for a prompt reply.

If I'm not mistaken, it was actually a primary field, not a mirror one. I'm now away from the code but I'll try to see to it later today and come up with a simple example.

@joaotavora
Copy link
Owner

I assumed it was a mirror field since the syntax for primary field transformations (if I myself am not mistaken) is

${1:initial-value$$(lisp-form-evaluated-on-field-entry-modification-and-exit)}

notice the double $$

@immerrr
Copy link
Contributor Author

immerrr commented Jun 4, 2013

Oh, ok, I reproduced the example in original post by heart and missed the initial field value that has been there. Here's an example to reproduce the behavior.

This snippet works as expected upcasing entered text and printing the message after each keystroke:

# -*- mode: snippet -*-
# name: foobar
# key: foobar
# --
${1:foobar$(progn (message "exiting foobar") (upcase text))}

This does print a message once, but the field contents remain the same:

# -*- mode: snippet -*-
# name: foobar
# key: foobar
# --
${1:foobar$(when yas-moving-away-p (progn (message "exiting foobar") (upcase text)))}

@immerrr
Copy link
Contributor Author

immerrr commented Jun 5, 2013

PS. There's also

Error during redisplay: (jit-lock-function 1) signaled (void-function font-lock-keyword-face)

in snippet-mode that is fixed for me with

diff --git a/yasnippet.el b/yasnippet.el
index 7e2eeee..f0cd8fd 100644
--- a/yasnippet.el
+++ b/yasnippet.el
@@ -808,8 +808,8 @@ Honour `yas-dont-activate', which see."
             ("${\\([0-9]+\\):?"
              (0 font-lock-keyword-face)
              (1 font-lock-warning-face t))
-            ("${" font-lock-keyword-face)
-            ("$[0-9]+?" font-lock-preprocessor-face)
+            ("${" . font-lock-keyword-face)
+            ("$[0-9]+?" . font-lock-preprocessor-face)
             ("\\(\\$(\\)" 1 font-lock-preprocessor-face)
             ("}"
              (0 font-lock-keyword-face)))))

Sorry, I'm too lazy right now to make a full-fledged pull request.

@lalopmak
Copy link

lalopmak commented Aug 4, 2013

I can confirm that this doesn't work during primary or mirrored transformations. I'm trying to make a quote bbcode that only leaves the = in [quote=author] if the user fills in the author field, otherwise changes to [quote]

# -*- mode: snippet -*-
# name: [quote=..] ... [/quote]
# key: quote
# group: bbcode
# --
[quote${1:$$(if (and yas-moving-away-p 
                     (equal "=" text)) 
                "" 
                (unless (eq ?= (string-to-char text)) 
                        (concat "=" text)))}]${2:quoted text}[/quote]$0

But the = fails to be cleared upon moving away from empty-author [quote=]. Upon adding messages:

# -*- mode: snippet -*-
# name: [quote=..] ... [/quote]
# key: quote
# group: bbcode
# --   
[quote${1:$$(if (and yas-moving-away-p 
                     (equal "=" text)) 
                (progn (message "moving condition") 
                       "") 
                (progn (message "staying condition") 
                       (unless (eq ?= (string-to-char text)) 
                               (concat "=" text))))}]${2:quoted text}[/quote]$0

It seems that, while the moving condition is being triggered, the "" is not being used.

Edit: I tried using this workaround:

# -*- mode: snippet -*-
# name: [quote=..] ... [/quote]
# key: quote
# group: bbcode
# --
[quote${1:$$(if (and yas-moving-away-p 
                     (equal "=" text)) 
                (let* ((field (overlay-get yas--active-field-overlay 'yas--field))
                       (start (marker-position (yas--field-start field))) 
                       (end (marker-position (yas--field-end field)))) 
                      (delete-region start end))
                (unless (eq ?= (string-to-char text)) 
                        (concat "=" text)))}]${2:quoted text}[/quote]$0

but, even though start and end are correct, delete-region doesn't seem to have any effect! Very puzzling!

@joaotavora
Copy link
Owner

@lalopmak you most definitely shouldn't be using delete-region inside the transformation. As I said, transformations, primary and secondary, are supposed to return text strings and have no side-effects. There are some edge cases where you might want a side effect, but I don't think this is one of them.

I'll have a deeper look at your example when I find the time, but from the looks of it it sounds much simpler. You seem to want a mirror of the author field that either returns a "=" or an empty string. You don't need a primary field transformation at all for that. just something like [quote${2:$(if yas-text "=" "")}${2:author}] (haven't tested it)

@lalopmak
Copy link

lalopmak commented Aug 5, 2013

Something like that was indeed my secondary solution, the problem being that the = doesn't appear unless they type something in. To improve upon that, I tried (without much success) to only make the equal disappear only when moving away from that field.

Though a missing = may not be the biggest deal in this case, a good usecase for this is when you have many initially-visible optional fields for the user to "tab through" quickly. In this case, he could just tab-tab-tab and type when appropriate, any fields in which he did not type automatically disappearing. Another, more general, usecase is if the user's final value just needs to be transformed into something else when he leaves.

Is it possible to add a "leaving hook" to a field, code to modify the text one last time?

@joaotavora
Copy link
Owner

@lalopmak isn't your objective that the field shows [quote] if the user pressed C-d to delete the author field is present or [quote=someauthor] if the user typed "someauthor TAB"?

@lalopmak
Copy link

lalopmak commented Aug 6, 2013

Aha, I've got it from that hint!

# -*- mode: snippet -*-
# name: [quote=..] ... [/quote]
# key: quote
# group: bbcode
# --
[quote${1:$$(if (and yas-moving-away-p 
                     (equal "=" text)) 
                (let ((field (overlay-get yas--active-field-overlay 'yas--field)))
                     (progn (yas--skip-and-clear field)
                            (yas-next-field 1)))
                (unless (eq ?= (string-to-char text)) 
                        (concat "=" text)))}]${2:quoted text}[/quote]$0

Basically copied the C-d syntax for when the user is "tabbing away" without any text. In terms of action, this is slightly more general than C-d in that the user can change his mind after typing something, delete it, and it'd still work. It's also slightly more convenient than C-d (especially if there are many fields to skip) because he can just keep using tab.

@joaotavora
Copy link
Owner

I still don't understand why this is better than the suggestion I gave you
two messages ago which consisted of a field and a mirror. You're just using
a field. Don't you see the utility in the mirror that conditionally shows
the "="? Do you want the user to type the "=" itself? Isn't that contrary
to the concept of boilerplate?

But it's OK.:-D, its a free world!

You are aware that none of this is guaranteed to work in the future though,
you're using way too much internals, and basically raping yasnippet :-D

@lalopmak
Copy link

lalopmak commented Aug 9, 2013

Our two examples have almost the same behavior. The only difference is that, with mine, the user still sees the = when the field is empty, whereas, with yours', he doesn't. No extra boilerplate in either case, just some (imo) additional clarity.

I would be very sad to learn that this has broken. A possible feature that could do the same thing (without exposing any additional internals) would be "leaving hooks". They could work in almost the same way as the mirror code, except that leaving code is only triggered when the user actually leaves the field. For example, let's say a mirror syntax was ${number:mirror code:leaving code}. Then all my junk above would be equivalent to a simple:

[quote${1:"=":(if (> (length yas-text) 0) "=" "")}$1]

That is, the mirror text is a constant "=", except when leaving the field, in which case the leaving code retains the "=" only if the field is non-empty. A similar format would work for more complicated examples:

<sometag ${1:"field1=":(if (> (length yas-text) 0) "field1" "")}$1  ${2:"field2=":(if (> (length yas-text) 0) "field2" "")}$2] ... >

Similarly to the quote's =, sometag would start with field1= and field2=, only deleting them if their respective texts ended up empty.


So far, this could've been done with C-d (as long as the user did not type anything beforehand). But C-d only has the capacity to delete fields upon leaving, not necessarily to transform them. Here's an example of a powerful snippet with "leaving transformation":

License: ${1:yas-text:(cond ((equal yas-text "") "None")
                            ((equal yas-text "gpl") "This program is free software: you can redistribute it and/or modify...")
                            ((equal yas-text "bsd") "Redistribution and use in source and binary forms...")
                            ((equal yas-text "mit") "Permission is hereby granted, free of charge...")
                            (t yas-text))}

So I guess this has turned into a proposal/feature request. I've tried looking at the code to see how I might put it in, but I admit I haven't been able to understand it well enough.

@joaotavora
Copy link
Owner

Lally Oppenheimer notifications@github.com writes:

Our two examples have almost the same behavior. The only difference
is that, with mine, the user still sees the = when the field is
empty, whereas, with yours', he doesn't. No extra boilerplate in
either case, just some (imo) additional clarity.

OK, I understand now. Maybe you don't know that the historical root of
yasnippet is the the TextMate editor, extremely popular in 2007/2008 and
even today. Many snippets that serve the function you need are
implemented in that editor like I suggested.

I don't know what language you're using, but I suspect that [quote=]
is sintactically incorrect, right? With my example, the snippet's
expansion is always valid code at every moment. That is the snippet
starts out with

[quote=DummyAuthor]

And DummyAuthor is highlighted. Because DummyAuthor doesn't really
exist the user can choose to type another author or type no author and
make it an authorless quote.

And this is the use case that yasnippet was designed to support, just
like TextMate did in its day.

You are right it is a question of opinion. However, by design,
yasnippet implements my opinion since I largely designed it :-). And I
designed it to be as close as possible to TextMate (which doesn't
support primary field transformations at all I think).

I have read your proposal and it will be registered here, but it has
slim chances. More syntax in yasnippet would be crazy, and making all
this field-side-effect mechanism work nicely with undo, for example,
is quite hard. That said, you are free to try and I wish you luck.

@joaotavora
Copy link
Owner

Just a last comment. In your last "License" example, have you noticed that you're proposing a new syntax for expanding abbreviations based on a few characters? Sounds familiar? Yes, you're reinventing yasnippet all over again!

But yasnippet already provides that with no additional syntax. You can enable yas-triggers-in-field. See http://stackoverflow.com/questions/9556922/how-can-i-expand-a-snippet-within-a-snippet-using-yasnippet

@immerrr
Copy link
Contributor Author

immerrr commented Oct 26, 2013

FTR here's what seems an another mention of this issue on stack overflow. The intention was to wrap yas-text of the primary field in parentheses upon leaving the field with something like that:

func ${1:$$(when (and yas-modified-p yas-moving-away-p) 
             (concat "(" yas-text ")"))} FuncName

And the observed behaviour was the same as in my request: the code was executed, but field value was not replaced.

@joaotavora
Copy link
Owner

@immerrr this seems to be a bug, albeit one I'm not keen on tacking in the near future, at least not with the current yasnippet engine, which is being reworked on the snippet-engine branch. Might I suggest you check my commente comment to Stefan's answer (http://stackoverflow.com/a/19602061/177259)

@immerrr
Copy link
Contributor Author

immerrr commented Oct 27, 2013

Rewrite is good news, count me in for waiting on its results :)

PS. looked into the code, especially tests, I like the ert-simulate-command function, I've been using (execute-kbd-macro (kbd ...)) for that purposes, but for separate commands your variant is probably shorter.

@joaotavora
Copy link
Owner

@immerrr would you be interested in writing a translator of textmate/yasnippet syntax into the new snippet.el syntax? I need a function that takes the string

printf ("${1:%s}\\n"${1:$(if (string-match "%" yas-text) "," "\);")  \
}$2${1:$(if (string-match "%" yas-text) "\);" "")}

And outputs a lisp list

("printf (\""
 (&field 1 "%s")
 "\\n\""
 (&mirror 1 (if (string-match "%s" field-text) "," "\);"))
 (&field 2)
 (&mirror 1 (if (string-match "%s" field-text) "\);" "")))

This is a relatively simple example, but if you're interested I can provide more examples/tips.

@immerrr
Copy link
Contributor Author

immerrr commented Nov 1, 2013

I'll try seeing to it over weekend, but I'm not sure I get where did the open-paren go in the example. (UPD: and all the non-field characters, for that matter)

@joaotavora
Copy link
Owner

Sorry, I made a mess of it. Should work now.

@immerrr
Copy link
Contributor Author

immerrr commented Nov 7, 2013

I'm trying to follow the doc here, but I can't see how would you like constructs like ${1:foobar$(upcase field-string)} to work in new snippets.

@joaotavora
Copy link
Owner

immerrr notifications@github.com writes:

I'm trying to follow the doc
here,
but I can't see how would you like constructs like ${1:foobar$(upcase field-string)} to work in new snippets.

Hi. Please assume that this behaviour (unimplemented for now) will be
given by a single (&transform FIELD-TRANSFORM) in he FIELD-DEFAULT
placeholder.

As to the contents of FIELD-TRANSFORM itself, it must be a lisp form
evaluating to "foobar" on expansion and (upcase field-string) every
other time. Assume you have a variable to distinguish between these
cases, and invent a name for that variable.

João

@immerrr
Copy link
Contributor Author

immerrr commented Nov 8, 2013

invent a name for that variable.

Ok, how about snippet-action variable taking one of the expand, transform, exit symbols as value?

@joaotavora
Copy link
Owner

immerrr notifications@github.com writes:

invent a name for that variable.

Ok, how about snippet-action variable taking one of the expand,
transform, exit symbols as value?

Reasonable enough (but should refer to the fields).

It's not the most important thing right now, don't worry about it. I
told you to assume it would be the same funcion but I might was well
decide later that a joint (&eval FORM) and (&transform FORM) on a
single such field is better.

I just want to get some "yas--parse-snippet (TEXTMATE-SNIPPET)"
translator into the yasnippet.el of the snippet-engine branch, buggy as
it might be, do you have something I can look at?

@immerrr
Copy link
Contributor Author

immerrr commented Nov 8, 2013

Sort of... Where do you want me to put it?

@joaotavora
Copy link
Owner

immerrr notifications@github.com writes:

Sort of... Where do you want me to put it?

Use github to fork yasnippet, then make it a commit to the
snippet-engine branch, and use the github web interface make a pull
request for

immerrr/yasnippet/snippet-engine
->
capitaomorte/yasnippet/snippet-engine

I'm pretty sure github allows this, otherwise send it via mail to
joaotavora [at] gmail.com.

J

@immerrr
Copy link
Contributor Author

immerrr commented Nov 8, 2013

Oh, thanks. I'm aware of git branching model, I was rather wondering which file would you like to see it in :)

@mathrick
Copy link

I've been wrestling with this for a while before finding this issue report, and I'm somewhat confused by the outcome. Is it currently at possible to have fields that automatically delete themselves when skipped without being edited? I also want placeholders and decorations on them. Specifically, I want a Ruby snippet which starts out as this(where >< denotes fields with placeholders):

def >name< (>args<)
   # $0 here
end

If I now press TABTAB, I should end up with this:

def name
   |
end

This is pretty much the question asked in the referenced stackoverflow post, and I'm aware of that. The reason I'm restating it is because there's no clear answer in here. I'd like an unambiguous explanation of whether it's possible to do in the current yasnippet engine, and if so, how to achieve it.

There's related functionality useful in writing code where it'd be nice to be able to generate a repeating placeholder list as long as the user edits them. Think of void foo (type1 arg1, type2 arg2, ...), where typeN argN (plus a comma) is added after each TAB until you skip one without editing, in which case the last-added placeholder is deleted along with the associated comma.

@joaotavora
Copy link
Owner

Maciej Katafiasz notifications@github.com writes:

This is pretty much the question asked in the referenced stackoverflow
post, and I'm aware of that. The reason I'm restating it is because
there's no clear answer in here. I'd like an unambiguous explanation
of whether it's possible to do in the current yasnippet engine, and
if so, how to achieve it.

I advise against using primary field transformations for this sort of
thing, they are fragile and not designed to support these use cases.

See the (admittedly outdated) documentation for the use cases they are
designed to support.

However, its possible to achieve something very close. The solution
doesn't lie in "fields that automatically delete themseolves when
skipped without being edited".

Use this snippet

def ${1:name} ${2:$(if (string-match "." yas-text) "(" "")}${2:args}${2:$(if (string-match "." yas-text) ")" "")}
   $0
end

And instead of pressing TAB twice, press TAB and C-d to delete the second
field and skip to the end

You can also write and use TAB TAB. If you start typing arguments they
will automatically appear surrounded by parenthesis.

def ${1:name} ${2:$(if (string-match "." yas-text) "(" "")}$2${2:$(if (string-match "." yas-text) ")" "")}
   $0
end

The reason that there is no direct answer sometimes is that the
formatulation of the question is not good. And I don't have the time to
guess the posters intentions, even if they are very legitimate.

Your formulation was very good, though. Fell free to add this answer to
the stackoverflow question if you feel its useful.

Also, for all practical matters regarding yasnippet directly, the
current engine redesign is an implementation detail and currently
completely invisible to the end user, you.

There's related functionality useful in writing code where it'd be
nice to be able to generate a repeating placeholder list as long as
the user edits them. Think of void foo (type1 arg1, type2 arg2, ...), where typeN argN (plus a comma) is added after each
TAB until you skip one without editing, in which case the
last-added placeholder is deleted along with the associated comma.

This is not supported. You might have some success with
yas-triggers-in-field and some "arg' subsnippet, that, when exited, leave
point just after another "arg" snippet trigger.

Maybe the new snippet engine will support this kind of thing, but
yasnippet will probably not.

What yasnippet syntax would you propose for this?

@Skrylar
Copy link

Skrylar commented Jul 19, 2015

I found a fix for this posted to the newsgroup years ago here: https://groups.google.com/forum/#!topic/smart-snippet/HEJ5JLIXhp8

Had to update it to use the newer namespaces but it mostly works. I'll toss a patch up on github when I've made sure it updates other mirrors correctly and have access to my github box again.

@joaotavora
Copy link
Owner

@Skrylar, just a heads up, you can of course propose such a thing in a pull request, but I doubt I'll merge it without some thorough reviewing of its implications. I've only very briefly skimmed the patch you make mention to, and I would rather prefer to revive the effort that is connecting yasnippet to the new snippet-engine branch, and work from there. But open the new pull request and I'll have a better look there.

@npostavs
Copy link
Collaborator

npostavs commented May 5, 2016

It appears that in yas-next-field the transform is evaluated with yas--eval-lisp, but the field is never updated. Is this intentional?

Looks like a simple bug to me, will be fixed in #689.

The rest of this thread started to branch out a lot and got kind of confusing, if anyone wants to follow up on something here, I suggest opening a new issue.

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