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

Allow operators in constant pattern #280

Merged
merged 1 commit into from
Aug 25, 2023
Merged

Conversation

wise0704
Copy link
Contributor

another one I missed

Copy link
Member

@JoeRobich JoeRobich left a comment

Choose a reason for hiding this comment

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

Thanks @wise0704!

@JoeRobich JoeRobich merged commit af5a34a into dotnet:main Aug 25, 2023
2 checks passed
@wise0704 wise0704 deleted the pattern branch August 25, 2023 08:59
@kasperk81
Copy link

@wise0704 are these related to https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/functional/pattern-matching? see or is not highlighted:

if (day is DayOfWeek.Monday or DayOfWeek.Wednesday) { }

@JoeRobich
Copy link
Member

@kasperk81 Where are you seeing or not being highlighted? I can see it is being given the correct textmate scope.
image

@kasperk81
Copy link

@JoeRobich i was using vscode and github. github has three months cycle, so their next release will capture the changes on dec 12 that were made here after sep 19 (their last release): github-linguist/linguist#6627. i'm not sure about vscode's cadence.

@JoeRobich
Copy link
Member

@kasperk81 For GitHub highlighting, you may also want to check with https://github.com/tree-sitter/tree-sitter-c-sharp as I know they are moving more languages away from the textmate grammars. This test in particular makes me wonder if it is supported.

@lildude
Copy link

lildude commented Dec 7, 2023

👋 from Linguist. I can confirm we do indeed use the tree-sitter grammar on GitHub.com as shown the this line in the vendor/README.md file for highlighting C# files.

We do however still ship this grammar with Linguist for other languages which might reference the source.cs scope in their grammar. With that in mind, whilst the C# highlighting won't be affected, other grammars might be affected by these errors which were picked up by our compiler yesterday:

- repository `vendor/grammars/csharp-tmLanguage` (from https://github.com/dotnet/csharp-tmLanguage) (4 errors)
  - Invalid regex in grammar: `source.cs` (in `grammars/csharp.tmLanguage`) contains a malformed regex (regex "`(?<!\.\s*)\b(await)\b`": lookbehind assertion is not fixed length (at offset 9))
  - Invalid regex in grammar: `source.cs` (in `grammars/csharp.tmLanguage`) contains a malformed regex (regex "`\G(?=(?~\*/)$)`": unrecognized character after (? or (?- (at offset 7))
  - Invalid regex in grammar: `source.cs` (in `grammars/csharp.tmLanguage`) contains a malformed regex (regex "`^(\s*+)(\*(?!/))?(?=(?~\*/)$)`": unrecognized character after (? or (?- (at offset 22))
  - Invalid regex in grammar: `source.cs` (in `grammars/csharp.tmLanguage`) contains a malformed regex (regex "`(?<!\.\s*)\b(await)\b`": lookbehind assertion is not fixed length (at offset 9))

These are likely to all be because the grammar compiler enforces PCRE rules because the old highlighting engine that uses this grammar uses PCRE for performance reasons. Feel free to ignore these and I'll keep tracking them as I do all the others in github-linguist/linguist#3924

@kasperk81
Copy link

@damieng would know better but it does seem to recognize or as binary_pattern in playground https://tree-sitter.github.io/tree-sitter/playground
image

assuming linguist update on dec 12 will take the latest tree-sitter-csharp i will just wait. thanks for the info! :)

@lildude
Copy link

lildude commented Dec 7, 2023

assuming linguist update on dec 12 will take the latest tree-sitter-csharp i will just wait. thanks for the info! :)

Linguist updates don't influence the treesitter grammar updates. They happen at their own cadence performed by the team that owns the highlighting service.

@kasperk81
Copy link

thanks @lildude. i can then only hope that it won't take long before they update the service. (unless someone can poke them) ;)

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

Successfully merging this pull request may close these issues.

4 participants