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

Dart: support digit separators #7105

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

srawlins
Copy link
Contributor

@srawlins srawlins commented Sep 5, 2024

Dart is adding support for digit separators, the underscore, like 1_000_000 or 0x33_33_33. The Dart playground, DartPad, still runs on codemirror5, so this would be a welcome fix.

@marijnh
Copy link
Member

marijnh commented Sep 5, 2024

That regexp seems to allow underscores anywhere, whereas the linked proposal only allows them between digits. I think this'll make _ tokenize as a number, which doesn't seem right.

@srawlins
Copy link
Contributor Author

srawlins commented Sep 5, 2024

As I understand it, the number pattern is only checked when we see numberStart, line 65 in clike.js. With manual testing, I see that _ is styled as a "variable."

As for other spots where _ is not allowed (e.g. in 1_._0, neither underscore is allowed), I think it is fine to allow them for the purposes of in-progress typing, and error-recovery. For example, in the Dart parser, we parse 1_._0 as a number literal, and also report an error. This reduces errors-carried-forward. And for a trailing _ like 100_, it is fine to allow it for code-in-progress.

If you think we should be more strict here and only parse error-free numbers as numbers, I can amend the PR.

@marijnh marijnh merged commit e1b414d into codemirror:master Sep 5, 2024
1 check passed
@marijnh
Copy link
Member

marijnh commented Sep 5, 2024

Oh, indeed, there's a separate regexp for the start of the number. Then this is close enough, indeed.

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.

2 participants