Skip to content

Implement \R, \v, \h for character/scalar modes #384

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

Merged
merged 10 commits into from
May 9, 2022

Conversation

natecook1000
Copy link
Member

This implements these meta characters as per the Unicode proposal. We still need to expand how we treat \n. It should have mostly the same behavior as \v, but I also want to make sure that /\r\n/ will match an "\r\n" character.

@natecook1000 natecook1000 requested review from hamishknight and Azoy May 6, 2022 16:36
@natecook1000
Copy link
Member Author

@swift-ci Please test

Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

Thanks! Do you also want to take #375 to fix the handling of \N?

default: return false
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Character has a isNewline. Can we share implementation and/or SPI for this?

Copy link
Member

Choose a reason for hiding this comment

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

Also, is this the same list?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the same list — we should ultimately have this on UnicodeScalar as well; probably with a name to match Character's.

@natecook1000
Copy link
Member Author

@swift-ci Please test

@natecook1000
Copy link
Member Author

@swift-ci Please test

@natecook1000
Copy link
Member Author

@swift-ci Please test

1 similar comment
@natecook1000
Copy link
Member Author

@swift-ci Please test

@natecook1000
Copy link
Member Author

@swift-ci Please test macOS platform

@natecook1000
Copy link
Member Author

@swift-ci Please test macOS platform

@natecook1000
Copy link
Member Author

Figured this issue out — we were looking at the character view to test whether the current position was the start of a line, even in Unicode scalar mode. I've fixed that and audited the rest of the assertions to make sure that we're looking at the correct view.

@natecook1000
Copy link
Member Author

@swift-ci Please test

@natecook1000
Copy link
Member Author

@swift-ci Please test

@natecook1000 natecook1000 merged commit c16e389 into swiftlang:main May 9, 2022
@natecook1000 natecook1000 deleted the newline_handling branch May 9, 2022 23:15
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.

4 participants