Skip to content

encoding.utf8: fix is_punct func - #22647

Merged
spytheman merged 5 commits into
vlang:masterfrom
felipensp:fix_utf8_is_punct
Oct 26, 2024
Merged

spytheman merged 5 commits into
vlang:masterfrom
felipensp:fix_utf8_is_punct

Conversation

@felipensp

@felipensp felipensp commented Oct 24, 2024 •

Copy link
Copy Markdown
Member

Comment thread vlib/encoding/utf8/utf8_util.v Outdated
Comment thread vlib/encoding/utf8/utf8_util.v Outdated
felipensp and others added 2 commits October 25, 2024 08:24
Co-authored-by: Delyan Angelov <delian66@gmail.com>
Co-authored-by: Delyan Angelov <delian66@gmail.com>
@felipensp
felipensp marked this pull request as ready for review October 25, 2024 13:56
Comment thread vlib/encoding/utf8/utf8_is_punct_test.v Outdated
Comment thread vlib/encoding/utf8/utf8_is_punct_test.v Outdated
Comment thread vlib/encoding/utf8/utf8_is_punct_test.v Outdated

import encoding.utf8

fn test_true() {

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.

Suggested change
fn test_true() {
fn test_is_punct() {

that will have all the cases for that specific public function will be a better scheme, so that if an assertion fails, the message will be more informative (the test function name is included in the failure)

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.

However, the assertions themselves are already much better and direct, so the test function name does not matter as much 🤷🏻‍♂️ .

@spytheman spytheman 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.

Good work.

@spytheman
spytheman merged commit 419645a into vlang:master Oct 26, 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.

! is not punct char

4 participants