Skip to content

Handle escaped separators in split()#128

Open
DmitrySharabin wants to merge 2 commits into
mainfrom
review/pr-127-fixes
Open

Handle escaped separators in split()#128
DmitrySharabin wants to merge 2 commits into
mainfrom
review/pr-127-fixes

Conversation

@DmitrySharabin

@DmitrySharabin DmitrySharabin commented May 25, 2026

Copy link
Copy Markdown
Member

Summary

split() honors \<sep> as an escaped separator: not split on, and the backslash is stripped from the yielded part. MapType.parseEntries() uses it for :-splitting.

Test plan

  • npx htest test/split.js --ci
  • npx htest test/PropType.js --ci

@DmitrySharabin DmitrySharabin mentioned this pull request May 25, 2026
4 tasks
@DmitrySharabin DmitrySharabin changed the title Fix missing type exports and escaped colon handling Fix missing type exports, escaped colon handling, and JSDoc types May 26, 2026
Comment thread src/plugins/props/types/basic.js Outdated

@LeaVerou LeaVerou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think since escaped colons are an older bug, they should remain separate. For the exports, I'm not sure this is the right fix. Can you commit the JSdoc fixes directly to types-refactor and slim this PR down to the colon handling fix?

LeaVerou added a commit that referenced this pull request May 26, 2026
Co-Authored-By: Dmitry Sharabin <dmitrysharabin@gmail.com>
@LeaVerou

Copy link
Copy Markdown
Contributor

Can you commit the JSdoc fixes directly to types-refactor

Done

@DmitrySharabin DmitrySharabin changed the title Fix missing type exports, escaped colon handling, and JSDoc types Fix escaped colon handling in parsed dictionary entries May 26, 2026
@DmitrySharabin

DmitrySharabin commented May 26, 2026

Copy link
Copy Markdown
Member Author

slim this PR down to the colon handling fix

Done. After the type refactors PR is landed, we'll retarget it to the rollback-signals branch (if it won't happen automatically).

@DmitrySharabin DmitrySharabin requested a review from LeaVerou May 26, 2026 20:30
@LeaVerou LeaVerou mentioned this pull request May 27, 2026
2 tasks
Base automatically changed from types-refactor to rollback-signals May 27, 2026 15:07
@LeaVerou LeaVerou changed the base branch from rollback-signals to main May 27, 2026 16:22
@netlify

netlify Bot commented May 27, 2026

Copy link
Copy Markdown

Deploy Preview for nude-element ready!

Name Link
🔨 Latest commit 3a603b4
🔍 Latest deploy log https://app.netlify.com/projects/nude-element/deploys/6a184b2bd2aa310007a25177
😎 Deploy Preview https://deploy-preview-128--nude-element.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@LeaVerou LeaVerou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think instead of handling this ad hoc, parseEntries() should be using split() which I believe handles escapes already (and if it doesn't, it should).

@DmitrySharabin DmitrySharabin changed the title Fix escaped colon handling in parsed dictionary entries Handle escaped separators in split() May 28, 2026
@DmitrySharabin DmitrySharabin requested a review from LeaVerou May 28, 2026 14:09
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