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 syntax for Zig #4005

Merged
merged 3 commits into from Jan 8, 2019
Merged

Add syntax for Zig #4005

merged 3 commits into from Jan 8, 2019

Conversation

@jfo
Copy link
Contributor

@jfo jfo commented Jan 27, 2018

Hi! I'd like to add syntax support for the Zig programming Language.

Checklist:

  • I am adding a new language.
    • The extension of the new language is used in hundreds of repositories on GitHub.com.
    • I have included a real-world usage sample for all extensions added in this PR:
    • I have included a syntax highlighting grammar.
    • I have included a change to the heuristics to distinguish my language from others using the same extension.

Closes ziglang/zig#138
Closes ziglang/sublime-zig-language#1

Please let me know and thank you!

@jfo jfo force-pushed the jfo:master branch from 8c16357 to e7421bf Jan 27, 2018
@andrewrk
Copy link

@andrewrk andrewrk commented Jan 29, 2018

LGTM

@jfo
Copy link
Contributor Author

@jfo jfo commented Feb 3, 2018

@andrewrk is Zig's original author and not a fly by rando LGTM'ing FWIW 😃

@andrewrk
Copy link

@andrewrk andrewrk commented Feb 11, 2018

what's the timeline for merges?

@lildude
Copy link
Member

@lildude lildude commented Feb 11, 2018

what's the timeline for merges?

From a quick look at the search results, I'm afraid it's going to be dependent on the growth of the use of Zig on GitHub.com and not based on anyone within the Linguist community.

Your search results show 517 files. If I remove the import keyword, this goes up to 635. If I then look into how the files of the more generous results are distributed across repos, I see they're spread across only 52 repos... which is a long way off our "used in hundreds of repositories on GitHub.com" requirement.

In short, Zig is not popular enough for this PR to be merged just yet and I've tagged this PR accordingly.

@jfo
Copy link
Contributor Author

@jfo jfo commented Feb 11, 2018

@lildude ok, thanks! I will update this PR with new search results as Zig grows. Is the changeset otherwise well-formed?

@lildude
Copy link
Member

@lildude lildude commented Feb 11, 2018

Yup, LGTM.

@Alhadis Alhadis mentioned this pull request Mar 1, 2018
2 of 2 tasks complete
@ice1000
Copy link

@ice1000 ice1000 commented Mar 16, 2018

image
The funnest post I've ever seen in GitHub issue comments, I saw this comment and laughed for an hour

@jfo jfo force-pushed the jfo:master branch 3 times, most recently from 15fc460 to a0796fc Apr 15, 2018
@andrewrk
Copy link

@andrewrk andrewrk commented Jun 1, 2018

I see they're spread across only 52 repos

How do you perform this search? I was unable to find this in the GitHub web interface.

@lildude
Copy link
Member

@lildude lildude commented Jun 1, 2018

How do you perform this search? I was unable to find this in the GitHub web interface.

I have a script that queries the search API. Unfortunately, I can't share it at the moment as it abuses the API and will result in rate limiting and blocking for other peeps.

@Alhadis has a written Harvester which does something similar, but within the browser so isn't subject to API abuse rate limiting.

@pchaigno
Copy link
Collaborator

@pchaigno pchaigno commented Jun 1, 2018

but within the browser so isn't subject to API abuse rate limiting.

As far as I know, it is constrained by the rate-limit as well, but there's a timer to slow down requests. I'm expecting this is okay according to the terms of service since:

Researchers may scrape public, non-personal information from GitHub for research purposes, only if any publications resulting from that research are open access.

@Alhadis
Copy link
Collaborator

@Alhadis Alhadis commented Jun 1, 2018

As far as I know, it is constrained by the rate-limit as well

It is. I had to slow it down on purpose:

// Throttle the next request so GitHub doesn't bite our head off
return wait(2000).then(() => next());
@pchaigno
Copy link
Collaborator

@pchaigno pchaigno commented Aug 4, 2018

Closing in favor of #4219. We'll monitor the popularity of the extension there.

If you have an improved search query compared to the one used in #4219, please post it here and I'll update #4219.

@pchaigno pchaigno closed this Aug 4, 2018
@andrewrk
Copy link

@andrewrk andrewrk commented Dec 1, 2018

It's time to re-open this issue. See #4219 (comment)

@andrewrk
Copy link

@andrewrk andrewrk commented Dec 3, 2018

@lildude could you please re-open this issue?

@andrewrk
Copy link

@andrewrk andrewrk commented Dec 5, 2018

cc @vmg

@daurnimator
Copy link

@daurnimator daurnimator commented Dec 13, 2018

Any blockers here? @pchaigno could you open this PR again?

@Alhadis Alhadis reopened this Dec 13, 2018
@ice1000
Copy link

@ice1000 ice1000 commented Dec 13, 2018

Any deprecation in the syntax/example file? I suppose Zig's design is changing during this time.

@jfo
Copy link
Contributor Author

@jfo jfo commented Dec 13, 2018

I'll update this soon. Thanks for reopening!

@jfo jfo force-pushed the jfo:master branch 2 times, most recently from 117e4ca to b57fbb7 Dec 15, 2018
@andrewrk
Copy link

@andrewrk andrewrk commented Dec 15, 2018

  1) Failure:
TestGrammars#test_readme_file_is_in_sync [/home/travis/build/github/linguist/test/test_grammars.rb:42]:
Grammar list is out-of-date. Run `script/list-grammars`.
--- expected
+++ actual
@@ -428,5 +428,5 @@
 - **YARA:** [blacktop/language-yara](https://github.com/blacktop/language-yara)
 - **YASnippet:** [Alhadis/language-emacs-lisp](https://github.com/Alhadis/language-emacs-lisp)
 - **Zephir:** [phalcon/zephir-sublime](https://github.com/phalcon/zephir-sublime)
-- **Zig:** [ziglang/sublime-zig-language](https://github.com/ziglang/sublime-zig-language)
+- **Zig:** [zig-lang/sublime-zig-language](https://github.com/zig-lang/sublime-zig-language)
 "

Looks like an easy fix. github.com/zig-lang was renamed to github.com/ziglang.

@jfo jfo force-pushed the jfo:master branch from b57fbb7 to ffcf880 Dec 15, 2018
@Alhadis
Copy link
Collaborator

@Alhadis Alhadis commented Dec 15, 2018

github.com/zig-lang was renamed to github.com/ziglang

Hyphenation™: Disrupting technology since 1958! :thumb-up:

Technically, redirects should be handled by the add-grammar script... but they're uncommon enough to warrant manual updates, instead of adding extra complexity to the script. Plus, it discourages authors from wanton hyphenation changes. 😉

Copy link
Collaborator

@Alhadis Alhadis left a comment

LGTM, regarding both changes and language popularity.

@ice1000
Copy link

@ice1000 ice1000 commented Dec 15, 2018

Wow! We're going to have syntax-highlighted Zig files on GitHub!

@ararslan
Copy link
Contributor

@ararslan ararslan commented Dec 22, 2018

Is there anything else that needs to happen before this can be merged?

@Alhadis
Copy link
Collaborator

@Alhadis Alhadis commented Dec 22, 2018

Yes, a final word of approval from GitHub staff.

/cc @lildude

@andrewrk
Copy link

@andrewrk andrewrk commented Jan 3, 2019

/home/travis/build/github/linguist/vendor/licenses/grammar/sublime-zig-language.txt:
  - cached license data out of date

what does this mean?

@jfo
Copy link
Contributor Author

@jfo jfo commented Jan 3, 2019

@andrewrk The submodule commit didn't match the cached license hash.

@lildude
lildude approved these changes Jan 8, 2019
@lildude lildude merged commit 589f7eb into github:master Jan 8, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ararslan
Copy link
Contributor

@ararslan ararslan commented Jan 8, 2019

🎉 🎉 🎉

When will this go live? The next time Linguist is tagged?

@lildude
Copy link
Member

@lildude lildude commented Jan 8, 2019

When will this go live? The next time Linguist is tagged?

Yup. I aim to make a release approximately once a month, though couldn't in December due to a deploy freeze over the Christmas break and because I took some time off 😄

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

Successfully merging this pull request may close these issues.

None yet

9 participants