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

Fix highlighting of base 10 numbers #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

asottile
Copy link

This was incorrectly highlighting the following:

ref: 3.8.3  # should be a string, highlighted as a number

This was incorrectly highlighting the following:

```yaml
ref: 3.8.3  # should be a string, highlighted as a number
```
@asottile
Copy link
Author

@infininight this patch is small and subtle, would you be able to review?

@geekley
Copy link

geekley commented Jun 18, 2022

Weirdly the dot in the regex is ... official? https://yaml.org/type/float.html

[-+]?([0-9][0-9_]*)?\.[0-9.]*([eE][-+][0-9]+)? (base 10)

This allows even weird things like 1.....1
Don't ask me why.

@asottile
Copy link
Author

I suspect it's a mistake in the spec there -- it's not implemented that way in libyaml

@geekley
Copy link

geekley commented Jun 18, 2022

In fact, it's highly questionable, since one of the examples doesn't even match the regex (and has a typo):

exponentioal: 685.230_15e+03

@geekley
Copy link

geekley commented Jun 18, 2022

I don't know whether those "type" pages should be "official" or not ... they're "drafts" and from a previous version of the spec.
The 1.2.2 spec has a different regex in the recommended Core Schema
[-+]? ( \. [0-9]+ | [0-9]+ ( \. [0-9]* )? ) ( [eE] [-+]? [0-9]+ )?
This all is too confusing. Same with what counts as a boolean.
I don't know much about YAML, but I already know I don't like it 😆

@geekley
Copy link

geekley commented Jun 18, 2022

Oh. I suspect the repeatable . was supposed to be a repeatable _. This makes sense based on that example, which would match in this case. I'll try reporting this on yaml spec repo.

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