-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 STAR language and .star file extension for Starlark #4840
Conversation
I don't think this pull request addresses the concerns expressed in #4759. The issue was not with the in-the-wild usage, but with the presence of many other |
Sorry, but if I am not mistaken, in linguist are many extension collisions,
that are resolved with the classifier. For example `.md`.
So all this files are not going to be identified as Starlark since a sample
is provided for the classifier. I am wrong?
Usually this can be a problem because this consume more resources than just
check the extension. But given the number of .star is slow, I don't believe
this is going to be a problem.
…On Sun, Apr 19, 2020, 9:47 PM Paul Chaignon ***@***.***> wrote:
I don't think this pull request addresses the concerns expressed in #4759
<#4759>. The issue was not with
the in-the-wild usage, but with the presence of many other .star files.
All these files will be recognized as Starlark files with the pull request
as is.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4840 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMAB6XTV43Z3ETYLJCPWRDRNNIOZANCNFSM4MF2JEBA>
.
|
That is correct, but requires us to also have support in Linguist for the conflicting languages. In the case of |
Well but if the other languages are not identified will remain the same,
but the Starlark files will be identified.
Even if doesn't work like this, the amount files that aren't Starlark it's
minimal. And Starlark is growing everyday.
Please reconsider it.
…On Sun, Apr 19, 2020, 10:52 PM Paul Chaignon ***@***.***> wrote:
Sorry, but if I am not mistaken, in linguist are many extension collisions,
that are resolved with the classifier. For example .md.
So all this files are not going to be identified as Starlark since a sample
is provided for the classifier. I am wrong?
That is correct, but requires us to also have support in Linguist for the
conflicting languages. In the case of .md, Linguist knows both GCC
Machine Description and Markdown and can use the classifier (and other,
more precise strategies) to distinguish between the two. So for .star,
we'll need to identity the conflicting languages and add them to Linguist
if they meet usage requirements.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4840 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMAB6SJLQKC67N5CDGSAQ3RNNQCRANCNFSM4MF2JEBA>
.
|
This pull request has been automatically marked as stale because it has not had recent activity, and will be closed if no further activity occurs. If this pull request was overlooked, forgotten, or should remain open for any other reason, please reply here to call attention to it and remove the stale status. Thank you for your contributions. |
This pull request has been automatically closed because it has not had activity in a long time. Please feel free to reopen it or create a new issue. |
@pchaigno I want to bump this issue. What will require for us to "identity the conflicting languages"? I made a search query to look for recently indexed *.star files, checked a few first pages and it seems majority of cases are Startlark cases. The rest is some data files or files without a particular pattern. |
bump! seconding @fkorotkov. /cc @lildude @pchaigno - any way we can get this looked at? |
Things are still in the same place as it was... you still need to identify the other users of the You don't have to identify every user of the |
Just for a reference we at Cirrus CI started using Starlark along side YAML for allowing to programmatically configure CI builds. So there will be more We also created a plugin for IntelliJ platform to support code assistance for |
@lildude Just so I understand, if we go through and identify all the conflicting languages that meet usage requirements, do they need to be added as new languages in this same PR? |
@rohansingh I think you may have missed the last sentence in my last response:
|
Sorry, reading comprehension failure. Anyway, sounds good, I'll try to get to this today. update: Unfortunately I don't see myself getting around to this after all. Just using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking this as still needing changes
I've done this. I've identified the other most commonly used language is Self-defining Text Archive and Retrieval commonly referred to as STAR. I've pushed changes to your repo to add support to this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That pro-dna.nmr.star
sample is huge (8,613 lines / 145 KBs). Sure we can't trim it down a bit? I noticed that a lot of sections repeat the same data.
🤦 And I point this out myself so often 😆 I'll get the ✂️ out. |
@Alhadis replaced it with a smaller sample. |
Description
This PR's adds
.star
as an extension to the Starlark language. The.star
extension is the default extension for Starlark scripts not related to Bazel, this is growing every day since many projects are embedding the language in their own projects.This extension is used in all official interpreters in the examples and as well in the testdata.
Also is the official extension recommended by many opensource projects:
This was discarded initially at #4759 (comment)
The amount of
.star
files currently is 30k, since the PR instrudicing the language the usage has growth a 50%.Checklist:
@lildude has also added support for STAR as it also has the
.star
extension which is very popular on GitHub: