Skip to content

Enhance split_string to support splitting on white space [TG-2922] #2071

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.

Sign up for GitHub

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 6 commits into from
Apr 19, 2018
Merged

Enhance split_string to support splitting on white space [TG-2922] #2071

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e87edbf
Removing wrong elements from the make file
Apr 12, 2018
252c24c
Migrate old string utils unit tests
Apr 12, 2018
07009d4
Refactored test to run all combinations
Apr 16, 2018
145f474
Adding tests for empty string edge cases
Apr 16, 2018
a385d9b
Allowed split_string to be used on whitespace if not also trying to s…
Apr 12, 2018
fc8ba88
Revert to aborting precondition for function inputs
Apr 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions unit/util/string_utils/split_string.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,39 @@ SCENARIO("split_string", "[core][utils][string_utils][split_string]")

run_on_all_variants(string, delimiter, expected_results);
}
GIVEN("An empty string")
{
std::string string = "";
WHEN("Splitting it")
{
expected_resultst expected_results;
expected_results.no_strip_no_remove = {""};
expected_results.strip_no_remove = {""};

// TODO(tkiley): This is probably wrong, since I'd expect removing empty
// TODO(tkiley): elements to return an empty vector here.
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree, we could change this to return an empty vector. I've looked at the places where split_string() is currently used, and they would also work when returning an empty vector (except an assertion assert(!result.empty()) would need to be removed in util/unwind.cpp). Also once changed, the current usages of split_string() can be slightly simplified as some of them have a check for this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Excellent. Since this PR is already a pre-req for another I will put together a separate PR to address this.

expected_results.no_strip_remove_empty = {""};
expected_results.strip_remove_empty = {""};

run_on_all_variants(string, ',', expected_results);
}
}
GIVEN("A whitespace only string")
{
std::string string = " ";
WHEN("Splitting it")
{
expected_resultst expected_results;
expected_results.no_strip_no_remove = {" "};
expected_results.strip_no_remove = {""};
expected_results.no_strip_remove_empty = {" "};
// TODO(tkiley): This is probably wrong, since I'd expect removing empty
// TODO(tkiley): elements to return an empty vector here.
expected_results.strip_remove_empty = {""};

run_on_all_variants(string, ',', expected_results);
}
}
}

SCENARIO("split_string into two", "[core][utils][string_utils][split_string]")
Expand Down