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

Introduce triple bang syntax to remove triggers #218

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

somini
Copy link

@somini somini commented Jan 4, 2016

In a snippets file, snippet!!! foo will remove the trigger from the
current lookup for the current context. Load order still matters.

This is useful to override a series of triggers with different
descriptions, and turn that into a single trigger, so that you don't
have to choose.

Similar, but different to #214.

@somini
Copy link
Author

somini commented Jan 4, 2016

Of course, this could be "solved" by forking the snippets repo, but #214 was solved in the same way of adding features to snipmate, so I think this is warranted.

@ajzafar
Copy link
Collaborator

ajzafar commented Jan 8, 2016

@somini I'm not against merging this, but I want to make sure I understand the intention first. Can you provide a detailed scenario (ideally with some sample file structure and snippets) of what you're trying to achieve?

My understanding is that if you're editing, say, your personal cpp.snippets, you want to a) remove instances of snippets in, for example, your copy of the honza snippet repo's cpp.snippets but not those in the repo's c.snippets that may be imported as a result of extends or scope aliases; and b) be able to remove specific snippets based on their description. Part (a) needs to be tweaked to work with the override option. Part (b) seems inflexible to me. I'm thinking maybe we allow an optional regex to be specified to determine exactly which snippets are to be removed. What do you think?

@somini
Copy link
Author

somini commented Jan 10, 2016

My use case when I created this was the following:
I'm using https://github.com/honza/vim-snippets unchanged.

https://github.com/honza/vim-snippets/blob/master/snippets/python.snippets#L94-L121
This file defines 3 different snippets for the "try" trigger. I wanted to override this and include a single one, so that expansion happens automatically. I don't want to affect other snippets with the try trigger.

https://github.com/honza/vim-snippets/blob/master/snippets/htmldjango.snippets#L41-L44
This file extends html. Both include the "for" trigger. In order for expansion to be automatic, I deleted the "for" trigger using this triple-bang syntax from the HTML snippets file ONLY, and keep the htmldjango intact.

As for your comments, the assumptions are correct:
a) I don't see how the override option changes the behaviour of this syntax. Do you mind ellaborate on that?
b) The regex idea is pretty good, so that you can have full control over what snippets to delete. I'll work on that ASAP.

@ajzafar
Copy link
Collaborator

ajzafar commented Jan 29, 2016

@somini Sorry for the really late response! The format of the path argument to the SetByPath() function changes depending on the setting of the override option. With it on, path is of the form scope<space>description. With it off, it's of the form scope<space>uniq_path<space>description, where uniq_path is a hopefully unique substring of the runtime path from which the snippet came. I was thinking instead of spaces to delimit the two to three parts of the function argument, we could decide on a delimiter. Then we could have the snippet!! directive take an optional regex that matches the entire thing. That should hopefully meet your requirements while being as flexible as can be.

I'm hoping to spend more time on SnipMate in the coming weeks, so I can hopefully get this sorted once I finish up the work on #207.

@somini
Copy link
Author

somini commented Mar 6, 2016

Rebased this onto the latest master.

In a snippets file, `snippet!!! foo` will remove the trigger from the
current lookup for the current context. Load order still matters.

This is useful to override a series of triggers with different
descriptions, and turn that into a single trigger, so that you don't
have to choose.
@somini
Copy link
Author

somini commented Jun 24, 2016

Rebased this onto the latest master.

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

2 participants