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

Add Glimmer language #6556

Closed
wants to merge 9 commits into from
Closed

Add Glimmer language #6556

wants to merge 9 commits into from

Conversation

gilest
Copy link
Contributor

@gilest gilest commented Sep 22, 2023

Description

Adds support for Glimmer.js which will be the component authoring format of the next Edition of Ember.js.

Checklist:

script/add-grammar https://github.com/lifeart/vsc-ember-syntax

Note: I removed `text.html.handlebars` from the `vsc-ember-syntax` in grammars.yml to (hopefully) avoid conficting with the existing Handlebars syntax which is also provided by `vsc-ember-syntax`.
Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from a quick scan but usage isn't high enough for either extension yet. I'll review this as I make each release.

vendor/licenses/git_submodule/vsc-ember-syntax.dep.yml Outdated Show resolved Hide resolved
@NullVoxPopuli
Copy link

The extension usage is counted individually? Even though they're basically the same?

That'd be like treating js and ts separately?
(For context, this pr is for glimmer flavored JavaScript and glimmer flavored typescript)

@lildude
Copy link
Member

lildude commented Sep 23, 2023

The extension usage is counted individually? Even though they're basically the same?

Yup. This is documented. Once a language has an extension that meets the usage requirements, an override can be used for the others until such time as they’re popular enough for inclusion.

That'd be like treating js and ts separately?
(For context, this pr is for glimmer flavored JavaScript and glimmer flavored typescript)

We do as Typescript is technically not JavaScript. They’re also considered two separate languages as they use two different syntax highlighting grammars. Your grammar repo does too 😉

@NullVoxPopuli
Copy link

NullVoxPopuli commented Sep 23, 2023

ok, thanks far clarifying. ❤️
I'm a little sad about this though, because the language grammars defer to to JS and TS for everything except for the stuff in between the <template> / </template> tags (which is the exact same for both extensions 💪 ).

We'll keep growing the adoption in the mean time tho!
I keep finding more folks every day that don't know about the new (approaching 2 years now (almost!)) syntax in our community. 🙃

@github-linguist github-linguist deleted a comment from XoL1507 Sep 29, 2023
@github-linguist github-linguist deleted a comment from XoL1507 Sep 29, 2023
@github-linguist github-linguist deleted a comment from XoL1507 Sep 29, 2023
@NullVoxPopuli
Copy link

NullVoxPopuli commented Sep 30, 2023

For those following along, an update, we have ~2.4k files (1.3k of the javascript (about the same) and now 1.1k of the typescript (+ 200))


made a template for future updates

gjs + gts gjs gts
2.3k 1.5k 1k

with markdown

gjs + gts w/ md gjs gts
2.7k 1.7k 1.1k

@wagenet
Copy link
Contributor

wagenet commented Oct 5, 2023

@lildude To clarify, it's 200 unique repos or 2,000 total files, right? How many unique repos are there so far?

@lildude
Copy link
Member

lildude commented Oct 5, 2023

@lildude To clarify, it's 200 unique repos or 2,000 total files, right? How many unique repos are there so far?

@wagenet your questions are answered by #5756 😁

@NullVoxPopuli
Copy link

For those following along, an update, we have ~2.6k files (1.6k of the javascript and now 1.1k of the typescript)

I'm not sure why querying for both files is less than the sum of the individual queries.


gjs + gts gjs gts
2.6k 1.6k 1.1k

with markdown

gjs + gts w/ md gjs gts
2.9k 1.8k 1.2k

@lildude
Copy link
Member

lildude commented Oct 9, 2023

I'm not sure why querying for both files is less than the sum of the individual queries.

I think this will probably be due to rounding of the numbers, but I'm only guessing; I'm not involved with the Search side of things.

As an aside, we really don't need these weekly updates. The PR won't be merged any quicker as I only merge feature additions when I make a new release which aligns approximately with the GitHub Enterprise Server releases. I also assess all pending popularity PRs at the same time.

There are also test failures that need addressing.

@NullVoxPopuli
Copy link

As an aside, we really don't need these weekly updates

fair enough! I was doing it more for the ember community, as they often ask -- but I suppose links to the queries is probably sufficient!

There are also test failures that need addressing.

@gilest can you take a look at those?

Because otherwise it fails the TestGrammars#test_readme_file_is_in_sync test.

Not sure why this URL is considered not in sync. Maybe because of `ace_mode: javascript`?
@gilest
Copy link
Contributor Author

gilest commented Oct 12, 2023

Given that

  • .gjs extension seems like it will meet the acceptance criteria sooner
  • we may want to model Glimmer JS and Glimmer TS as two separate languages

I'm considering opening a new PR targeting Glimmer JS only

@gilest
Copy link
Contributor Author

gilest commented Oct 12, 2023

Closed in favour of #6578

Will stand up a separate PR for the TS language later on

@gilest gilest closed this Oct 12, 2023
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants