linguist icon indicating copy to clipboard operation
linguist copied to clipboard

Add Noir

Open critesjosh opened this issue 1 year ago • 30 comments

Description

Adding the Noir programming language.

Checklist:

  • [x] I am adding a new language.
    • [x] The extension of the new language is used in hundreds of repositories on GitHub.com.
      • Search results for each extension:
        • https://github.com/search?type=code&q=NOT+is%3Afork+path%3A*.nr+%2F%5E%5Csfn%5B+%5Ct%5D%2B%5B%5E%5C%28%5Cs%5D.%3F%5C%28%2F+NOT+user%3AAztecProtocol+NOT+user%3Anoir-lang
    • [x] I have included a real-world usage sample for all extensions added in this PR:
    • [x] I have included a syntax highlighting grammar: https://github.com/noir-lang/vscode-noir
    • [x] I have added a color
      • Hex value: #2f1f49
      • Rationale: This color is used in the Noir branding color palate.
    • [x] I have updated the heuristics to distinguish my language from others using the same extension.

critesjosh avatar May 25 '23 19:05 critesjosh

I'm not sure what is going on with Github search, but there are fewer files found today than yesterday. There should be more--I pushed a public repo and I know for a fact at least 3 other people did as well.

critesjosh avatar May 26 '23 17:05 critesjosh

@lildude Do you need anything else from me on this?

I am wondering how to best track popularity. When I first drafted this search query, there were 376 files. Now its only showing 358. I know more people have been pushing public repos, so it doesn't seem to be picking up on them for some reason.

critesjosh avatar Jun 01 '23 15:06 critesjosh

I am wondering how to best track popularity. When I first drafted this search query, there were 376 files. Now its only showing 358. I know more people have been pushing public repos, so it doesn't seem to be picking up on them for some reason.

There's only search, which is expected to fluctuate as people add and remove repos and files. This is only really noticeable as there are so few files. When things reach close to our requirements, a deviation of 18 files is trivial and not really an issue.

lildude avatar Jun 01 '23 16:06 lildude

I think you might have put the samples/Noir/roff.nr sample in the wrong directory or accidentally added it as it looks more like Roff than Noir to my naïve eye.

The same applies for the samples/Noir/roff1.nr sample which is also massive and waaay bigger than we need. ProTip: if the diff suppresses the content, the file is too big.

Do you need examples of Roff, since it has the same extension as Noir? Should I add them to samples/Roff/roff.nr? Or since there is already a samples/Roff/trekmanual.nr, can I delete them?

critesjosh avatar Jun 01 '23 16:06 critesjosh

I am wondering how to best track popularity. When I first drafted this search query, there were 376 files. Now its only showing 358. I know more people have been pushing public repos, so it doesn't seem to be picking up on them for some reason.

There's only search, which is expected to fluctuate as people add and remove repos and files. This is only really noticeable as there are so few files. When things reach close to our requirements, a deviation of 18 files is trivial and not really an issue.

Just curious, how do you assess when things reach close to your requirements, since the search doesn't return the number of repos?

critesjosh avatar Jun 01 '23 16:06 critesjosh

Do you need examples of Roff, since it has the same extension as Noir? Should I add them to samples/Roff/roff.nr? Or since there is already a samples/Roff/trekmanual.nr, can I delete them?

Noir and Roff are very different so the classifier shouldn't have problems distinguishing between the two. That said, there's no harm adding one (the smaller) to the Roff samples directory. As we'll then have two for each language, the "Classifier cross-validation (pull_request)" test will then start to test the classifier for the .nr extension.

Just curious, how do you assess when things reach close to your requirements, since the search doesn't return the number of repos?

See https://github.com/github-linguist/linguist/issues/5756 (this is referenced in the docs too). I'm slowly reassessing this now the new search is open for all and we can get better results. When I've come up with a good reliable strategy, I'll update the docs and close that issue.

lildude avatar Jun 01 '23 16:06 lildude

Thanks for your help with this! Let me know if I need to make any more updates.

critesjosh avatar Jun 01 '23 18:06 critesjosh

I've fixed the errors I can, however I can't fix the issue with your grammar. I've removed the submodule and then attempted to re-add it and it fails with:

$ script/grammar-compiler add https://github.com/noir-lang/vscode-noir
latest: Pulling from linguist/grammar-compiler
Digest: sha256:fcaf08a6e27d3d88212d9ca8aceee30d4089499a77bb05307871febd8073790f
Status: Image is up to date for linguist/grammar-compiler:latest
docker.io/linguist/grammar-compiler:latest
WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
1 errors found in new grammar 'repository `https://github.com/noir-lang/vscode-noir`':
- Grammar conversion failed. File `vscode-noir` failed to parse: grammars: unsupported extension ''

Compilation failed. Aborting
$

This is because your grammar doesn't have files as expected by TextMate and similar editors.

We expect the certain extensions in specific directories:

https://github.com/github-linguist/linguist/blob/8a2415b96ad796af16e853ddb091521af919c486/tools/grammars/compiler/loader.go#L119-L129

The src/noir.tmLanguage.json file in the repo doesn't match any of these.

You'll need to get the grammar maintainers to restructure their repo or find another grammar.

When you come to add the grammar again, you can run script/add-grammar and then change the tm_scope line in languages.yml to the scope name from the grammar.

lildude avatar Jun 02 '23 10:06 lildude

The vscode-noir repo has been updated to put the file at syntaxes/noir.tmLanguage.json, but I am still seeing this error when running script/grammar-compiler add https://github.com/noir-lang/vscode-noir.

1 errors found in new grammar 'repository `https://github.com/noir-lang/vscode-noir`':
- Grammar conversion failed. File `vscode-noir` failed to parse: grammars: unsupported extension ''

Compilation failed. Aborting

Any suggestions?

critesjosh avatar Aug 31 '23 15:08 critesjosh

Works for me if I use the documented command 😉 (on a clean branch):

➜ script/add-grammar https://github.com/noir-lang/vscode-noir.git
Checking Docker is installed and running
Registering new submodule: https://github.com/noir-lang/vscode-noir.git
Cloning into '/workspaces/linguist/vendor/grammars/vscode-noir'...
remote: Enumerating objects: 156, done.
remote: Counting objects: 100% (50/50), done.
remote: Compressing objects: 100% (30/30), done.
remote: Total 156 (delta 28), reused 25 (delta 15), pack-reused 106
Receiving objects: 100% (156/156), 159.46 KiB | 3.13 MiB/s, done.
Resolving deltas: 100% (63/63), done.
latest: Pulling from linguist/grammar-compiler
32fb02163b6b: Pull complete 
[...]
e2fa649b33f6: Pull complete 
Digest: sha256:fcaf08a6e27d3d88212d9ca8aceee30d4089499a77bb05307871febd8073790f
Status: Downloaded newer image for linguist/grammar-compiler:latest
docker.io/linguist/grammar-compiler:latest
OK! added grammar source 'vendor/grammars/vscode-noir'
        new scope: source.nr
Caching grammar license
Caching dependency records for linguist
  git_submodule
    Using AL (2d704ea4d7e8aa32fc4b415f61d29412ed9e7fcf)
[...]
  * 443 git_submodule dependencies
Updating grammar documentation in vendor/README.md
➜ 
➜ git st
## master...origin/master
MM .gitmodules
 M grammars.yml
A  vendor/grammars/vscode-noir
?? vendor/licenses/git_submodule/vscode-noir.dep.yml
➜ 

lildude avatar Aug 31 '23 17:08 lildude

:face-palm: got it, thank you :pray:

critesjosh avatar Aug 31 '23 19:08 critesjosh

Thanks a lot for the help and reviews @lildude & @Alhadis!

Any further changes we could help address to get this ready for a re-review?

Savio-Sou avatar Sep 01 '23 05:09 Savio-Sou

Error updating vendor/grammars/vscode-noir
error: pathspec 'vendor/grammars/vscode-noir' did not match any file(s) known to git

You forgot to commit the vendor/grammars/vscode-noir directory after running script/add-grammar:

➜ git st
## master...origin/master
MM .gitmodules
 M grammars.yml
A  vendor/grammars/vscode-noir    <----- THIS ONE
?? vendor/licenses/git_submodule/vscode-noir.dep.yml
➜ 

lildude avatar Sep 01 '23 10:09 lildude

I ran it again and it looks like I was missing an update to vendor/README.md and the license file (vscode-noir.dep.yml) as well, so I pushed those too.

critesjosh avatar Sep 01 '23 14:09 critesjosh

Looking good from a quick look and the tests are all passing 🎉. I'll make a final review when the popularity has reached our documented levels.

lildude avatar Sep 01 '23 15:09 lildude

Wouldn't it be better to use the tree-sitter plugin rather than the vscode grammar?

hhamud avatar Sep 10 '23 17:09 hhamud

Wouldn't it be better to use the tree-sitter plugin rather than the vscode grammar?

Nope because Linguist has no control over the choice of tree-sitter grammars used (we don’t accept them in this repo) and Noir is also not a very popular language so wouldn’t be considered by the grammar engine team. The VSCode grammar is the best and quickest way to get syntax highlighting once Noir is popular enough for inclusion.

lildude avatar Sep 10 '23 18:09 lildude

@lildude what do you think, are we getting close to mergability?

We have many repos with only one .nr file and some with many and we are about halfway between the 200-2000 range. I've been checking this recently and the number has been fluctuating quite a bit, the other week it was over 1k.

critesjosh avatar Oct 20 '23 00:10 critesjosh

We've just made some changes to the Noir grammar which this PR points to.

@critesjosh can you update the git submodule to the latest commit please?

TomAFrench avatar Nov 18 '23 11:11 TomAFrench

We've just made some changes to the Noir grammar which this PR points to.

@critesjosh can you update the git submodule to the latest commit please?

done!

critesjosh avatar Nov 21 '23 17:11 critesjosh

Our query is hitting 2k! Can we merge this now?

critesjosh avatar Feb 27 '24 15:02 critesjosh

The submodule could probably do with a bump btw @critesjosh. We've made some changes to the highlighting recently.

TomAFrench avatar Feb 27 '24 16:02 TomAFrench

@lildude how are we doing with this integration? There are more and more people exploring zero-knowledge proofs so it would be really cool to have support 😊

signorecello avatar Mar 01 '24 08:03 signorecello

Still got failing tests. Run this cmd to fix. I'll re-evaluate popularity when I come to make the next release which will in the next two weeks, assuming the failures have been addressed by then.

lildude avatar Mar 01 '24 10:03 lildude

Thanks! Just ran the command and pushed changes

critesjosh avatar Mar 04 '24 15:03 critesjosh

Things don't look popular enough once we exclude the AztecProtocol user that is responsible for nearly 650 of the found files.

lildude avatar Mar 12 '24 11:03 lildude

Huh, I only see a difference of ~200 files (1.9k vs 1.7k), but I guess we can wait a few more weeks. :slightly_smiling_face:

image

critesjosh avatar Mar 12 '24 17:03 critesjosh

Hitting 1.8k again, when is a new release planned @lildude ?

signorecello avatar Apr 09 '24 09:04 signorecello

Same as usual... approx 3-4 months after the last one.

lildude avatar Apr 09 '24 09:04 lildude