Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Adding syntax highlighting support for Liquid tags #145

Open
fusion809 opened this issue Mar 26, 2016 · 32 comments
Open

Adding syntax highlighting support for Liquid tags #145

fusion809 opened this issue Mar 26, 2016 · 32 comments

Comments

@fusion809
Copy link

Hi,

I use this package quite frequently when writing posts in my Jekyll site (The Hornerysource) and it would help big time if this package had syntax-highlighting and autocompletion support for Liquid tags. If it helps, there is a language-liquid package available for Atom, it has syntax-highlighting support for HTML files with Liquid tags in it.

Thanks for your time,
Brenton

@burodepeper
Copy link

I think this is best solved by adding markdown files to the filetypes recognized by the language-liquid package (see: https://github.com/kieranmasterton/language-liquid/blob/master/grammars/html%20(liquid).cson#L1-L4). This way language-liquid can inject its functionality into other grammars. (cc @kieranmasterton)

Another solution would be to include language-liquid as part of language-gfm (this is how I support html in my language-markdown package), but I'm not sure if that gives any problems for people that don't have the liquid package installed.

@fusion809
Copy link
Author

@burodepeper Ah, I just tried that and it failed. See instead the GFM syntax highlighting was removed entirely from the md file I was editing and language-liquid was used in its place.

@burodepeper
Copy link

@fusion809 Ah yes, you are probably right; of source it only uses a single grammar at the time. My bad.

It shouldn't be too difficult to add it similar to this: https://github.com/burodepeper/language-markdown/blob/master/grammars/language-markdown.json#L1500-L1506
Do you have experience with this? Otherwise I'll add it to language-markdown and then they/you can copy my approach from there.

@fusion809
Copy link
Author

Na I'm afraid my knowledge of programming is limited. So that would be a big help, thanks.

@burodepeper
Copy link

No problem. Could you create an issue over here and supply me with some liquid content so I can test if it works?

@fusion809
Copy link
Author

Shall do.

@fusion809
Copy link
Author

@burodepeper Thanks, it's just is there anyway to translate your language-markdown changes into config.cson changes to get language-gfm to use language-liquid syntax highlighting for Liquid tags? Your package is pretty good, but what can I say I'm a language-gfm fan.

@puranjayjain
Copy link

@burodepeper I have got a list of snippets related to liquid and would like to add snippets for them. where should I send the PR?

@burodepeper
Copy link

@puranjayjain If they're purely related to liquid, I'd say send the PR to https://github.com/kieranmasterton/language-liquid

@burodepeper
Copy link

burodepeper commented May 4, 2016

@fusion809 You can create a PR that adds this little bit to the language-gfm grammar. Not literally btw, the grammar only needs to include the text.html.liquid grammar and you're set.

@puranjayjain
Copy link

@burodepeper thanks but that repository seems to be dead as there is no update for the past 11 months, some issues and PRs are still open. What can be done about that? Any other suggestions or help would be appriciated

@burodepeper
Copy link

@puranjayjain Have you tried contacting the maintainer for the possibility of transferring ownership to yourself? According to the package.json the code is released under the MIT license, so if @kieranmasterton (ping, anyone home?) remains unresponsive, you could fork the project, and it release it as a new package.

@puranjayjain
Copy link

@burodepeper I had contacted the maintainer on his email id but seems there was no response. I'll try to contact him on twitter, if not I'll fork it and start a new package (which I'm not really sure if I want to).
If I do start it can I add you and @fusion809 to it as co-maintainers i won't be very available for it all the time so.

@burodepeper
Copy link

@puranjayjain I hope it works out via Twitter. I agree that forking and publishing as a new package is more of a last resort. I'm not personally invested in liquid, but I don't mind helping out. It looks rather straight forward.

@fusion809
Copy link
Author

My understanding of CoffeeScript and JS is so limited, especially when it comes to Atom packages, that I wouldn't be of much help. I'm more helpful in suggesting features than anything else. I wouldn't even know how to implement the necessary changes to this package (I know @burodepeper gave me that file link to indicate something about what needs to be done, but my knowledge of Atom packages isn't really enough for me to know what to do with that file I have tried to fork this package locally and work on it but I just don't understand CoffeeScript/JS enough to really follow its code).

@burodepeper
Copy link

@fusion809 @puranjayjain I don't mind maintaining the package if the two of you aren't comfortable with that.

@puranjayjain
Copy link

@burodepeper @fusion809 I'm waiting for the author's response and am giving him a grace period of 24 hours to respond after which I'll start a fork and post here for any updates or ask you to start a fork and that's it I guess for now.

@burodepeper
Copy link

@puranjayjain 24 hours is a bit short. Give it at least a week. Forking only directly helps yourself and isn't in the interest of the already established user group.

@puranjayjain
Copy link

@burodepeper you are right I'll give him a week's period, sorry for the hassle 😥

@burodepeper
Copy link

@puranjayjain No problem. Just actively trying to avoid unnecessary stress ; ) We'll talk later.

@fusion809
Copy link
Author

fusion809 commented May 20, 2016

@puranjayjain @burodepeper Unless I'm mistaken it's been two weeks, anyone gonna start working on an improvement for this package? Sorry if I come across as bossy, it's just these features would really help me at least and I doubt I'm the only one that would benefit from them.

@puranjayjain
Copy link

@fusion809 sorry for taking so long i'm creating a fork of the language liquid package and adding @burodepeper to it and then publishing it to atom.

@fusion809
Copy link
Author

@puranjayjain You sure you should have forked language-liquid and not language-gfm? As you're adding Liquid support to the language-gfm package, not making adjustments to the language-liquid package.

@puranjayjain
Copy link

@fusion809 Sure since it is part of liquid language extending the gfm sytax. We'll include gfm in it to extend the support and to gain advantage of color highlighting.
Similar to a lot of packages like template packages e.g language-nunjucks which adds support for nunjucks but on top of html

@puranjayjain
Copy link

@fusion809 I have left out some prs so you can add what you think is relevant. Now moving the conversation to the relevant repo.

@burodepeper
Copy link

burodepeper commented May 20, 2016

@puranjayjain No, I think @fusion809 is correct. For his request, you don't need to fork language-liquid. That's a separate issue. Just create a PR for language-gfm that includes language-liquid (you can copy this from language-markdown) and you are set for now.

You don't want to include language-gfm in language-liquid.

@puranjayjain
Copy link

@burodepeper @fusion809 ok can you review my latest commit there and tell me if I can go ahead with a PR here? i'm only talking about the snippet file

@burodepeper
Copy link

@puranjayjain Sure, if you create a PR for language-gfm first.

@fusion809
Copy link
Author

fusion809 commented May 20, 2016

Precisely, sorry for being unclear, I honestly thought we were all on the same page.

@puranjayjain
Copy link

@fusion809 @burodepeper I was just now added to the original repo of language-liquid so it is now being maintained again.

@burodepeper
Copy link

@fusion809 @puranjayjain created a PR in #154.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants