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

Added Angular (text.html.ng) support #10

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

Conversation

egesu
Copy link

@egesu egesu commented Oct 5, 2017

It already works when I put text.html.ng in additional grammars, however I thought this option is for optional extensions like backend languages. text.html.ng is always a frontend code, and no problem to put it in defaults.

By the way, text.html.ng is from language-html-angular package.

@liuderchi
Copy link
Owner

liuderchi commented Oct 5, 2017

@egesu additional grammars is not designed for backend languages only;
it's a flexible, experimental config for grammars supported by community packages.
By doing so we keep scope of ide-html not too big.

IMO suitable config for text.html.ng is either


How do you think ---

  • adding angular support as a checkbox config here
  • adding description about requirement for this config: package language-html-angular
    • e.g. you need to install language-html-angular to apply text.html.ng grammar support
    • by doing so a new comer can understand whether he or she needs to toggle the checkbox.

@liuderchi liuderchi mentioned this pull request Nov 6, 2017
@liuderchi
Copy link
Owner

@egesu any feedback to my comments?

@egesu
Copy link
Author

egesu commented Dec 4, 2017

Sorry about the delay. Additional grammars solves the issue very well, however the problem is when I wanted to use this feature, I had to find the grammar name for language-html-angular package. I didn't know anything about the grammars before, so I had to spend some time on it. Probably for a newbie in web development, additional grammars won't solve the issue. So I think checkbox would be nice.

I will update the PR as soon as possible.

@liuderchi
Copy link
Owner

... So I think checkbox would be nice.

@egesu That sounds Great!

IMO the description need to be informative by telling user that
this checkbox requires atom package language-html-angular

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

Successfully merging this pull request may close these issues.

2 participants