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

Add doesNot[Start/End]WithWhitespace methods to CharSequence assertions #3441

Closed
wants to merge 7 commits into from

Conversation

yyytir777
Copy link
Contributor

Check List:

@onacit
Copy link
Contributor

onacit commented Apr 25, 2024

Can't we use CharSequence#codePoints()?

Character.isWhiteapce(actual.codePoints().findFirst().getAsInt());
Character.isWhiteapce(actual.codePoints().reduce((v1, v2) -> v2).getAsInt());

@yyytir777
Copy link
Contributor Author

Thanks for better method. I'll fix it right now.

@joel-costigliola
Copy link
Member

@onacit why would using code points be better than char to detect whitespaces ?
do you have an example where using code points would detect whitespaces that char don't ?

@joel-costigliola joel-costigliola self-requested a review May 1, 2024 22:39
@onacit
Copy link
Contributor

onacit commented May 2, 2024

@joel-costigliola As documented in Character.isWhitespace(char), it'll, I think, be better when we use codePoints.

Note: This method cannot handle supplementary characters. To support all Unicode characters, including supplementary characters, use the isWhitespace(int) method.

Thanks.

@scordio
Copy link
Member

scordio commented May 2, 2024

Can't we use CharSequence#codePoints()?

Character.isWhiteapce(actual.codePoints().findFirst().getAsInt());
Character.isWhiteapce(actual.codePoints().reduce((v1, v2) -> v2).getAsInt());

Best would be to compose a test that demonstrates the need for codePoints, for example using Unicode whitespace that wouldn't be detected by Character.isWhitespace(char) but would otherwise work with Character.isWhitespace(int).

The test would also prevent regressions if we changed the implementation to Character.isWhitespace(char).

Taking inspiration from https://stackoverflow.com/a/18169122/9714611, maybe a value from this list could help?

@joel-costigliola
Copy link
Member

I'm on it @scordio ! @yyytir777 you'll get full credit for the PR don't worry :)

@joel-costigliola
Copy link
Member

Integrated thanks @yyytir777 !

@scordio scordio changed the title Add doesNot[Start/End]WithWhitespace methods to AbstractCharSequenceAssert Add doesNot[Start/End]WithWhitespace methods to CharSequence assertions May 4, 2024
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.

Add AbstractCharSequenceAssert#doesNotHaveAny(Leading|Trailing)Whitespaces
4 participants