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

Do not interpret characters that cannot be parsed in octal as int #176

Merged
merged 1 commit into from
Oct 25, 2023

Conversation

conao3
Copy link
Contributor

@conao3 conao3 commented Oct 24, 2023

ref: #152.

Some test cases expect 1.1, even though they should use yaml 1.2 by default. Confusing, but this is the minimum patch that does not change the current behaviour.
Perhaps the 1.2 syntax should be used, but that should be done in a separate PR.


ref: (yaml1.1 grammar)
https://yaml.org/type/int.html
https://yaml.org/type/float.html

@kislyuk kislyuk merged commit d4faa16 into kislyuk:develop Oct 25, 2023
3 of 17 checks passed
@kislyuk
Copy link
Owner

kislyuk commented Oct 25, 2023

Thanks!

@conao3 conao3 deleted the feature/152 branch October 26, 2023 00:10
@kislyuk
Copy link
Owner

kislyuk commented Apr 15, 2024

@conao3 this change breaks the parsing of numbers and I am reverting it (see #187). Can you please describe how you came up with these regular expressions and how they correspond to the YAML spec and to what is used in PyYAML?

kislyuk added a commit that referenced this pull request Apr 15, 2024
@conao3
Copy link
Contributor Author

conao3 commented Apr 16, 2024

from my comment, https://yaml.org/type/int.html has regular expressions

Resolution and Validation:
Valid values must match the following regular expression, which may also be used for implicit tag resolution:

 [-+]?0b[0-1_]+ # (base 2)
|[-+]?0[0-7_]+ # (base 8)
|[-+]?(0|[1-9][0-9_]*) # (base 10)
|[-+]?0x[0-9a-fA-F_]+ # (base 16)
|[-+]?[1-9][0-9_]*(:[0-5]?[0-9])+ # (base 60)

And https://yaml.org/type/float.html has below one.

Regexp:

 [-+]?([0-9][0-9_]*)?\.[0-9.]*([eE][-+][0-9]+)? (base 10)
|[-+]?[0-9][0-9_]*(:[0-5]?[0-9])+\.[0-9_]* (base 60)
|[-+]?\.(inf|Inf|INF) # (infinity)
|\.(nan|NaN|NAN) # (not a number)

And in incorporating these, I made some changes to meet all of the existing tests.

My personal opinion is that if you don't like the parsing of pyyaml, changing parser like ruamel.yaml or something else is another option.
But sorry for breaking the existing use case.

@Atemu Atemu mentioned this pull request Apr 18, 2024
13 tasks
Atemu added a commit to NixOS/nixpkgs that referenced this pull request Apr 18, 2024
Contains an important fix to number parsing

See kislyuk/yq#176
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