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

Change test files to be .luau #179

Closed
wants to merge 3 commits into from
Closed

Change test files to be .luau #179

wants to merge 3 commits into from

Conversation

Baileyeatspizza
Copy link
Contributor

as the file extension .luau is now supported it only seems fitting to change the test files to reflect that

I did a quick test and this seems to work fine
Screenshot 2021-11-08 at 17 55 56
please ignore the error it was reported in #171 but it shows that the file that errored is in fact luau 🥳

if you think I changed some code where I shouldn't have please suggest a change or if you have write access just remove it!

this change isn't exactly necessary but it helps to start building an ecosystem of .luau soon we could replace every .lua file in this repository with .luau

This shouldn't be a quick change please take time to ensure everything is working properly.

revert bench.py as all the benchmark files use .lua
@zeux
Copy link
Collaborator

zeux commented Nov 8, 2021

As mentioned on the relevant issue, we should take small steps to adopt the new extension. Right now merging this will mean that we lose syntax highlighting, language attribution on GitHub, quality of editor integration in various editors, etc., for all of these files.

Please be patient; we can take a look at renaming the internal files to use .luau extension once these change, but I don't think it's the right time yet.

@Baileyeatspizza
Copy link
Contributor Author

ah I understand

@zeux
Copy link
Collaborator

zeux commented Nov 8, 2021

I'm going to close this for now. We may come back to this in the future, but for now this change would result in some issues on our end and may get difficult to merge across the two repositories so it would be better if we handled this internally.

@zeux zeux closed this Nov 8, 2021
@CompeyDev
Copy link

Heya, I believe it is possible to include syntax highlighting for a language on GitHub, by simply contributing to GitHub's syntax highlighting library, "linguist".

https://github.com/github/linguist/blob/master/CONTRIBUTING.md

@vegorov-rbx
Copy link
Collaborator

Heya, I believe it is possible to include syntax highlighting for a language on GitHub, by simply contributing to GitHub's syntax highlighting library, "linguist".

That would be nice to add.
I'm not ready to make it myself right now (even though I have experience in TextMate grammars), so maybe some next time or someone else in the team/community.

@CompeyDev
Copy link

That would be nice to add.
I'm not ready to make it myself right now (even though I have experience in TextMate grammars), so maybe some next time or someone else in the team/community.

Was figuring the same, I personally do not have much experience here, but I'll go looking for possible contributors within the Roblox OSS Community.

@CompeyDev
Copy link

An update on this, I think I'll be able to implement it myself, seems pretty trivial... But before I do so, I'd need to know:

  1. What the preferred color for the language would be? (#00a2ff, perhaps?)
  2. As for the group parameter, is luau considered a child language of lua?
# group                 - Name of the parent language. Languages in a group are counted
#                         in the statistics as the parent language.

Lastly, we would need to add luau's grammar to ace, which is the syntax highlighting library that linguist uses.

@CompeyDev
Copy link

UPDATE: This would have to be put on hold for the timebeing as it looks like GitHub has temporarily changed the language criteria for eligible languages.

See github-linguist/linguist#5756.

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

Successfully merging this pull request may close these issues.

4 participants