Skip to content

fix: quote YAML 1.1 ambiguous strings - #202

Open
Vasundhara117 wants to merge 1 commit into
codercops:developfrom
Vasundhara117:fix/yaml-1-1-scalar-quoting
Open

Vasundhara117 wants to merge 1 commit into
codercops:developfrom
Vasundhara117:fix/yaml-1-1-scalar-quoting

Conversation

@Vasundhara117

Copy link
Copy Markdown

What and why

Quote strings that YAML 1.1 parsers may interpret as numbers, booleans, timestamps, or merge-key syntax. Add regression tests for the specified edge cases while preserving unquoted output for ordinary strings.

Closes #145

How I tested this

  • npx vitest run lib/__tests__/json-utils.test.ts — passed
  • npm run lint — 0 errors; 13 warnings in existing, unrelated files
  • npm run test — 381 tests passed
  • npm run build — passed

AI assistance

Used ChatGPT to help interpret the issue requirements, reason through edge cases, and review the implementation and tests.

Type of change

  • Bug fix
  • Tests

Checklist

  • I was assigned the linked issue
  • npm run lint && npm run test && npm run build passes locally
  • Logic changes live in lib/ and have a test in lib/__tests__/
  • For a bug fix: the new test fails without my change
  • No analytics, trackers, or calls that send user data off-device
  • Works in both light and dark themes (if UI changed)
  • Inputs have labels and it works with the keyboard alone (if UI changed)
  • New tool: registry entry, page.tsx, client component, opengraph-image.tsx, and the bug report dropdown are all done

Notes for reviewers

The tests verify YAML output text directly without adding a YAML parser dependency.

@anurag629 anurag629 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @Vasundhara117, this is close. Everything listed in #145 is quoted now, and the ones that should stay bare still are. I ran all of them through PyYAML 6.0, js-yaml and yaml in 1.1 mode, as values and as keys, and each one comes back as the same string. On top of develop, lint, all 381 tests and the build pass, the new tests fail on develop's json-utils.ts, and taking out any one of your new checks fails a test.

One gap, partly my fault for writing "between the digits" in the issue. YAML 1.1 parsers take _ anywhere after the first digit, and a base 60 number can end in a dot, so PyYAML still reads these back as numbers (yaml in 1.1 mode too):

  • 1__000 and 1000_ give 1000
  • 1.0_0 gives 1.0, and .5_0 gives 0.5
  • 0b_101 and 0b101_ give 5
  • 1_0:30 gives 630, and 1:30. gives 90.0

Your hex check already allows _ anywhere after 0x. Could you do the same in the decimal, binary and base 60 checks, and add these to the quoted list in the test? _1, 0b2 and the other bare ones should stay as they are.

Small thing while you're in there: the extra blank line at line 155 of the test file can go.

This branch has not been deployed

No deployments
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.

Quote YAML strings that YAML 1.1 parsers read as numbers, booleans or merge keys

2 participants