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

[Feat request] Avoid assigning multiple snippet sources to a single language #88

Closed
lambtho12 opened this issue Oct 12, 2021 · 18 comments
Closed

Comments

@lambtho12
Copy link
Contributor

In my opinion, it would be better to avoid linking a single language to multiple snippet files.
For instance, snippets for markdown are loaded from global.json, markdown.json and vscode-jekyll-snippets.json. In my personal experience, I do not use jekyll. Therefore, the jekyll-specific snippets are just irrelevant; worse, they even conflict with some custom snippets I have set.

I believe (and so is the developer of luasnip) that it should be the job of the snippet engine to mix the various files according to the desired filetype/framework and not the job of the source to pre-mix them.

What do you think of it?

@rafamadriz
Copy link
Owner

Yes, you're absolutely right. In fact I've been thinking about this for a long time.

I never liked the idea of mixing snippets from other frameworks. As you mentioned with Jekyll, the same happens with other snippets that are from a specific frameworks.

What I'd like to do is having an option to enable extra snippets for a specific frameworks or whatever.

Right now there's a couple of filetypes that have snippets not relevant to the vanilla language. Like for example:

  • ruby which also has snippets for rails.
  • html has for ejs and Vue.
  • JavaScript also includes Vue snippets.
  • markdown has Jekyll.

All of that is irrelevant for people not using those frameworks. And should not be included by default. I want all of those to only be enabled through an option, here's the thing, I've no idea on how to approach it. This "plugin" is only json files and I dont know what to do so certain snippets are only available through an option.

Any ideas are welcome!

@lambtho12
Copy link
Contributor Author

lambtho12 commented Oct 18, 2021

The way I see it, this repository should be considered only as a snippet source dictionary. Therefore it should only provide snippets related to the language itself by default and provide separately framework snippets (rails, jekyll, vue, etc). Then it should be the job of the snippet engine (LuaSnip for instance) to mix them or provide an option to load multiple snippet files for a given filetype.

A fix may be to use a specific field in the package.json to identify framework snippets (and keep the language field in order to specify the related language).

{
  "framework": "jekyll",
  "language" : "markdown",
  "path": "./snippets/jekyll/vscode-jekyll-snippets.json"
 },

Then snippets engine could only load snippets without the framework field by default and provide an option to also load specific framework ones.

As a bonus, by keeping the language field, we could hope that this change would not break a lot of snippet engines that are currently using this repo.

@L3MON4D3 (dev of LuaSnip) or maybe @leiserfg, do you have a better idea?

@leiserfg
Copy link
Contributor

In vim a file can have several ft, one option could be to use jekyll as the language of the snippet and then set the filetype as
ft=markdown.jekyll in the modeline of the files requiring it.

@L3MON4D3
Copy link

I'd agree with setting the language to the framework in the json, most snippet engines have some way of mixing filetypes for a given buffer, so that should work for most users.
framework would be nice to have if a framework has binding to multiple languages, but in that case the language could also be set to framework_language, which would be easier because language is already defined and recognized.

@rafamadriz
Copy link
Owner

okay I can add the framework key to the snippets that need it. @L3MON4D3 what would be the easiest for you ? you say you would be okay with the framework, but you also mention framework_language .

Also, should it be a boolean "framework": true or a string "framework": "jekyll"

@L3MON4D3
Copy link

I think easiest would be to just reuse language, that way ft=markdown.jekyll would work ootb on most snippet engines.

@rafamadriz
Copy link
Owner

that way ft=markdown.jekyll would work ootb on most snippet engines.

doesn't that messes up with syntax highlighting ?

@L3MON4D3
Copy link

Oh, I meant if ft is already set to that, it'd work. Luasnip also provides a separate mechanism to search in multiple sources (languages), if frameworks are listed as languages, eg. luasnip.filetype_extend("html", {"jekyll"}) would work.

@rafamadriz
Copy link
Owner

oh I see

if frameworks are listed as languages

So if I understand correctly, the language field is going to be reuse. And I need to add the framework to the language filed "language": ["markdown", "jekyll"], right ?

@L3MON4D3
Copy link

We add snippets defined with that language to markdown as well, "language": "jekyll" would add them to the jekyll-(vim-)filetype only.

@rafamadriz
Copy link
Owner

Sorry, I might look I bit stupid but I'm not 100% sure of what exactly I need to do. For example, just adding jekyll to markdown under language ("language": ["markdown", "jekyll"]) would be enough ? If I understand correctly, LuaSnip already has a mechanism to handle multiple sources. So with this, those extra snippets from frameworks are not going be added by default, and users would need to explicitly tell LuaSnip to load, say: jekyll snippets to markdown for example.

@L3MON4D3
Copy link

No problem :)

If language is set to ["markdown", "jekyll"], luasnip would add these snippets to the sources for ft=markdown and ft=jekyll (which would be suboptimal, jekyll-specific snippets would show up in regular markdown files).

For a file containing only jekyll-snippets, language should be set to "jekyll", so that users could, if they edit a markdown file that makes use of jekyll and would therefore like to expand jekyll-snippets, set the ft of that file to markdown.jekyll(or make luasnip search markdown and jekyll snippets via some other mechanism).

@rafamadriz
Copy link
Owner

rafamadriz commented Nov 3, 2021

Okay I see, thanks for explaining.

or make luasnip search markdown and jekyll snippets via some other mechanism

What would be the other mechanism ? my problem with setting ft to markdown.jekyll for example, is that messes up wit syntax highlighting, in markdown might not be a big deal since is pretty simple, but what if you want to do the same with javascript, adding react snippets with javascript.react. That's why I wonder what is the other way of doing it.

You mean this mechanism luasnip.filetype_extend("html", {"jekyll"}) ?

@L3MON4D3
Copy link

L3MON4D3 commented Nov 3, 2021

Yeah, that's the one.
Works globally though, so not a complete replacement to changing ft

@rafamadriz
Copy link
Owner

Works globally though, so not a complete replacement to changing ft

How globally ?

@L3MON4D3
Copy link

L3MON4D3 commented Nov 3, 2021

For example: two markdown-buffers, do luasnip.filetype_extend("markdown", {"jekyll"}) in one.
That would lead to jekyll-snippets being available in both buffers, not only the one the command was run in.
With ft that problem wouldnt occur, it's something that should probably be fixed.

@rafamadriz
Copy link
Owner

Okay thanks for your time, I'll make the necessary changes. I'll also add instructions in README, so people know how to add extra snippets from frameworks.

Downside is that this will only work wit luasnip, but I think is the best choice, I really don't like having a bunch of irrelevant snippets added by default.

@L3MON4D3
Copy link

L3MON4D3 commented Nov 3, 2021

Nice, that's great to hear :D
Looks like vsnip also has a comparable mechanism (doc), so vsnip-users are also covered.

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

No branches or pull requests

4 participants