dalingmeng commented on code in PR #263:
URL: https://github.com/apache/paimon-cpp/pull/263#discussion_r3913362764
##########
src/paimon/common/utils/string_utils_test.cpp:
##########
@@ -209,6 +210,21 @@ TEST_F(StringUtilsTest, TestIsNullOrWhitespaceOnly) {
auto ret = StringUtils::IsNullOrWhitespaceOnly(str);
ASSERT_TRUE(ret);
}
+ ASSERT_TRUE(StringUtils::IsNullOrWhitespaceOnly(u8"\u3000\u2000"));
+}
+
+TEST_F(StringUtilsTest, TestIsBlank) {
+ const std::vector<std::string> blank_strings = {
+ "", " ", " ", "\t", "\n", "\r",
+ "\r\n", " \t\n\r ", u8"\u1680", u8"\u2000", u8"\u3000", u8"
\t\u3000\u2000\n"};
+ for (const std::string& blank : blank_strings) {
+ ASSERT_TRUE(StringUtils::IsBlank(blank)) << blank;
+ }
+
+ ASSERT_FALSE(StringUtils::IsBlank("user1"));
+ ASSERT_FALSE(StringUtils::IsBlank(" user1 "));
+ ASSERT_FALSE(StringUtils::IsBlank(u8"\u00a0"));
+ ASSERT_FALSE(StringUtils::IsBlank(std::string("\xc0\x80", 2)));
Review Comment:
\u00a0 covers the non-breaking case nicely. Could we add the ones the
implementation is structurally most exposed on? IsJavaWhitespace skips U+2007
by splitting 0x2000–0x200a into 0x2000–0x2006 + 0x2008–0x200a, but the test
only exercises \u2000 — merging those two ranges back into one would keep every
test green while silently diverging from Character.isWhitespace, and
IsNullOrWhitespaceOnly would carry that into partition path computation.
> ```suggestion
> ASSERT_FALSE(StringUtils::IsBlank(std::string("\xc0\x80", 2)));
> // Non-breaking or otherwise excluded by Character.isWhitespace.
> ASSERT_FALSE(StringUtils::IsBlank(u8"\u2007")); // FIGURE SPACE
> ASSERT_FALSE(StringUtils::IsBlank(u8"\u202f")); // NARROW NO-BREAK
SPACE
> ASSERT_FALSE(StringUtils::IsBlank(u8"\u0085")); // NEL
> ASSERT_FALSE(StringUtils::IsBlank(u8"\u180e")); // not whitespace
since Java 8
> ```
>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
