Conversation
|
Holding this here for a short while. I need to add additional tests in other parsers/funcs |
TomWright
changed the title
TomWright
force-pushed
the
fix-yaml-int-parsing
branch
from
09642a1 to
01434f5
Compare
| return "", model.NewFloatValue(f), nil | ||
| case unstable.Integer: | ||
| i64, err := strconv.ParseInt(string(n.Data), 10, 64) | ||
| i64, err := strconv.ParseInt(string(n.Data), 0, 64) |
There was a problem hiding this comment.
I'm not an expert in TOML, but I took a quick look at the spec
- Please add tests for these:
Valid
42, -42, 0x12, 0o7, 0b111, 12_000, -12_000
Valid but as string: "042"
Invalid
-0x12, -0o7, -0b111, 042
All these values are valid with base 0 with ParseInt but not in TOML
Here are also feedbacks provided by an LLM
# ==========================================================
# HEXADECIMAL, OCTAL, AND BINARY BASES
# ==========================================================
# --- Valid (Case Insensitivity for Prefixes and Values) ---
hex_lowercase = 0xff12 # Standard hex
hex_uppercase = 0XFF12 # Uppercase prefix and values are VALID
bin_lowercase = 0b1101
bin_uppercase = 0B1101 # Uppercase 'B' is VALID
oct_lowercase = 0o755
oct_uppercase = 0O755 # Uppercase 'O' is VALID
# --- Invalid ---
# Negative signs are NOT supported for non-decimal bases
invalid_hex = -0xFF12
invalid_oct = -0o755
invalid_bin = -0b1101
# ==========================================================
# SCIENTIFIC NOTATION (FLOATS)
# ==========================================================
# --- Valid ---
standard_sci = 1e6 # 1,000,000
positive_exp = 5e+10 # 50,000,000,000
negative_exp = -1.5E-4 # -0.00015 (Uppercase 'E' is VALID)
large_float = 6_022.140e23 # Avogadro's number
# --- Invalid ---
# Decimal points must always have digits on both sides
invalid_point_1 = .1e2
invalid_point_2 = 1.e2
# Underscores are NOT allowed in the exponent part
invalid_underscore = 1e1_0
…sion - Handle underscores in YAML decimal integers (e.g. 1_000) - Handle signed prefixed numbers in YAML (+0x10, -0o10, etc.) - Add TOML test coverage for hex, octal, binary, and underscore numbers - Revert parse_literal.go to base 10 to prevent 010 being parsed as octal
|
Good additions. Is there anything else you want to change? Should I wait before reviewing again? |
I think that's all I've got for the time being. If you see any major issues please flag them, otherwise I'm happy to get the main issues resolved and follow-up with minor fixes for specific cases at a later date. It should be ready for you, thank you! |
2 participants
Fixes #525