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 MLIR grammar #4610

Merged
merged 4 commits into from
Aug 19, 2019
Merged

Add MLIR grammar #4610

merged 4 commits into from
Aug 19, 2019

Conversation

jpienaar
Copy link
Contributor

@jpienaar jpienaar commented Aug 15, 2019

Add support for MLIR

Description

Add support for MLIR.

Checklist:

Copy link
Contributor

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

LGTM! The tests failures are unrelated to this pull request, they work fine for me locally.

@jpienaar Could you update the template with the link to the exact sample file you used?

@jpienaar
Copy link
Contributor Author

@pchaigno Thanks! I combined it from multiple of the unit tests to get something a bit more interesting mix of types (all from the same repo and with same license). Would you prefer if I use one/more exact file instead?

@jpienaar
Copy link
Contributor Author

Updated template to point to exact file instead.

@pchaigno
Copy link
Contributor

I combined it from multiple of the unit tests to get something a bit more interesting mix of types (all from the same repo and with same license).

Thanks for the explanation. Including a few files instead of a single one would have been fine too.

@jpienaar
Copy link
Contributor Author

I combined it from multiple of the unit tests to get something a bit more interesting mix of types (all from the same repo and with same license).

Thanks for the explanation. Including a few files instead of a single one would have been fine too.

Sounds good to me, added a few more (exact samples from real usage). Let me know if anything else is needed. Thanks

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.

The test failure here is legit:

/home/runner/work/linguist/linguist/vendor/licenses/grammar/mlir-grammar.txt:
  - cached license data out of date

You'll need to run script/licensed (which should update the version in the license) and commit the updated file. You might need to manually add back the license text though, so feel free to.

@jpienaar
Copy link
Contributor Author

The test failure here is legit:

/home/runner/work/linguist/linguist/vendor/licenses/grammar/mlir-grammar.txt:
  - cached license data out of date

You'll need to run script/licensed (which should update the version in the license) and commit the updated file. You might need to manually add back the license text though, so feel free to.

Updated, that did it thanks

@lildude lildude merged commit 5c87522 into github-linguist:master Aug 19, 2019
@jpienaar
Copy link
Contributor Author

This seems to have stopped working (previously highlighted file no longer works), I don't see anything in the commit logs that would indicate a change though.

@lildude
Copy link
Member

lildude commented Oct 3, 2019

I've taken a look into this as this appears to only affect languages or grammars added in v7.6.0. Work is underway to improved the syntax highlighting service used by GitHub and I suspect it has missed the v7.6.0 update. I've opened an issue to bring this to the attention of the team responsible for the improvements.

I'll update when I know more or when things are working as expected again.

@lildude
Copy link
Member

lildude commented Oct 3, 2019

Work is underway to improved the syntax highlighting service used by GitHub and I suspect it has missed the v7.6.0 update.

All sorted. This was indeed the case.

@jpienaar
Copy link
Contributor Author

jpienaar commented Oct 3, 2019

Work is underway to improved the syntax highlighting service used by GitHub and I suspect it has missed the v7.6.0 update.

All sorted. This was indeed the case.

Great, thanks!

@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.

None yet

3 participants