Skip to content

C#: Added char token#3270

Merged
RunDevelopment merged 1 commit intoPrismJS:masterfrom
RunDevelopment:issue3170-cs
Dec 5, 2021
Merged

C#: Added char token#3270
RunDevelopment merged 1 commit intoPrismJS:masterfrom
RunDevelopment:issue3170-cs

Conversation

@RunDevelopment
Copy link
Copy Markdown
Member

@RunDevelopment RunDevelopment commented Dec 5, 2021

No description provided.

@RunDevelopment RunDevelopment mentioned this pull request Dec 5, 2021
Open
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 5, 2021

JS File Size Changes (gzipped)

A total of 1 files have changed, with a combined diff of -5 B (-0.2%).

Details
file master pull size diff % diff
components/prism-csharp.min.js 2.43 KB 2.43 KB -5 B -0.2%

Generated by 🚫 dangerJS against 5d167f4

hoonweiting
hoonweiting reviewed Dec 5, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@hoonweiting hoonweiting left a comment

Choose a reason for hiding this comment

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

What a coincidence! I was looking at this exact same thing a couple of hours ago too.

I did have a question though, and that is, should character have the quantifier {1,8} at the end?

prism/components/prism-csharp.js

Lines 81 to 84 in baa95ca

// strings & characters
// https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/lexical-structure#character-literals
// https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/lexical-structure#string-literals
var character = /'(?:[^\r\n'\\]|\\.|\\[Uux][\da-fA-F]{1,8})'/.source; // simplified pattern

I ask because I remember you asked me to remove it from #3105. I'm not familiar enough with C# (not that I knew Elm either!), so should the quantifier be removed here to make it consistent with Elm, or be left as it is for some special reason?

@RunDevelopment
Copy link
Copy Markdown
Member Author

RunDevelopment commented Dec 5, 2021

I used {1,8} to allow unfinished characters for a better typing experience. This is also the reason why I asked you to change Elm's char quantifier.

I though [0-9a-fA-F]+ was the best choice for Elm because many other languages with a \u{0000} escape syntax allow arbitrarily long hex numbers to represent the code point. I don't know much about Elm, so I assumed it's the same here (Elm docs aren't clear about this).

C# on the hand only allows \x00, \u0000, and \U00000000 as hex escape sequences. So 2, 4, or 8 hex digits. Since the maximum is 8, I went with {1,8} instead of +.

@hoonweiting
Copy link
Copy Markdown
Contributor

hoonweiting commented Dec 5, 2021

Ahh I see, thank you for the explanation! This PR looks good to me!

@RunDevelopment RunDevelopment merged commit 220bc40 into PrismJS:master Dec 5, 2021
@RunDevelopment RunDevelopment deleted the issue3170-cs branch December 5, 2021 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@hoonweiting hoonweiting hoonweiting left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement language-definitions needs review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@RunDevelopment @hoonweiting