Adds initial scaffolding for the split interface by aryann · Pull Request #2 · bemanproject/str_split

aryann · 2026-05-10T00:43:27Z

The purpose of this PR is to start the discussion on the interface shape. Note the TODOs I've added in the code for some of the decisions we need to make.

We can either get this PR to a good shape and merge it, or merge it earlier and evolve it via future PRs.

coveralls · 2026-05-10T00:45:48Z

Coverage Report for CI Build 25686618867

Warning

No base build found for commit 26f82af on main.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 100.0%

Details

Patch coverage: 12 of 12 lines across 1 file are fully covered (100%).

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →

Coverage Stats


Relevant Lines:	12
Covered Lines:	12
Line Coverage:	100.0%
Coverage Strength:	8.0 hits per line

💛 - Coveralls

bartholomaios

We should have an overload that takes an output iterator (like std::format and std::format_to).

I think the simplest case should output to std::vector without needing the "say" vector.

Fundamentally, I see 3 parts to this library:

Delimiter/Pattern concept with implementations for all the common cases.
A range adapter for more general use-cases that has better ergonomics than the current std::ranges::views::split.
Algorithms that eagerly evaluate for simple use-cases. This should include splitting to tuple-like.

bartholomaios · 2026-05-10T14:43:05Z


+#include <string>
+#include <string_view>
+


Should these go here? Should they be guarded by BEMAN_STR_SPLIT_USE_MODULES()?

It looks like we are the first ones to use the exemplar, and I am not 100% sure where these includes should go.

The namespace in todo.hpp is fully within the else branch of the BEMAN_STR_SPLIT_USE_MODULES, which is confusing to me (that would mean the TODO code will not be included when modules are enabled).

More broadly, we need to better understand the purpose of todo.hpp (we'll need to delete it eventually, but it's not clear to me why it was included in the first place).

It looks to me like all our headers need to:

Act like normal headers when modules aren't being used. Need to #include other headers in this case.

Contain just import beman.str_split when modules are enabled, except when included from the interface unit.

When included in the interface unit (str_split.cppm), expose the code, but rely on the import std; for standard library components.

Therefore (I think) the correct thing to do is:

#include <beman/str_split/config.hpp> #if BEMAN_STR_SPLIT_USE_MODULES() && !defined(BEMAN_STR_SPLIT_INCLUDED_FROM_INTERFACE_UNIT) import beman.str_split; #else #if BEMAN_STR_SPLIT_USE_MODULES() // includes here #endif namespace beman::str_split { // implementation here } // namespace beman::str_split #endif

Here's a good example to follow: https://github.com/bemanproject/utf_view/blob/main/include/beman/utf_view/to_utf_view.hpp

We'll want to guard std:: includes using !BEMAN_STR_SPLIT_USE_MODULES(). Our own headers should be included in the else branch of BEMAN_STR_SPLIT_USE_MODULES() && !defined(BEMAN_STR_SPLIT_INCLUDED_FROM_INTERFACE_UNIT).

bartholomaios · 2026-05-10T15:00:29Z

+            // Ensures this constructor does not hijack copy and move construction which would fail to compile with a
+            // difficult-to-read wall of errors.
+            different_from<Range, split_by>)
+    constexpr explicit split_by(Range&& range) : delimiter_(std::ranges::begin(range), std::ranges::end(range)) {}


I think we could constrain this with: std::constructable_from<std::string, std::from_range_t, R>. We wouldn't need the std::string_view constructor in that case.

bartholomaios · 2026-05-10T15:05:35Z

+    constexpr explicit split_by(std::string_view delimiter) : delimiter_(delimiter) {}
+
+    // Constructor for the single character case.
+    constexpr explicit split_by(char delimiter) : delimiter_(1, delimiter) {}


I would prefer the single char case to be a separate delimiter (like abseil and std::string::find).

split_by_char?

Also, see my comment about alternatives below and see if that's an approach you'd want to take instead.

Added split_by_char.

One thing we need to consider: should this be parameterized by CharT (with default == char) given that the inputs allow any char type?

bartholomaios · 2026-05-10T15:26:18Z

I'm going to take a first-pass at the range adapter today.

aryann · 2026-05-10T17:12:10Z

I replied to some of your comments, but not all. I won't have time to work on this today. Do you want to merge this PR and make your changes on top? I agree with all your comments, so no objections on making the changes.

Please also see my TODO on modeling this as a series of function overloads. That's another direction that may produce better ergonomics and names.

bartholomaios · 2026-05-11T01:48:43Z

Here's how it could look without the implicit conversion operators. This is modeled after std::format_to and std::ranges::to.

template<class OutputIt, class CharT, class Traits, class Delimiter>
auto str_split_to(OutputIt to, std::basic_string_view<CharT, Traits> str, Delimiter&& delim) -> OutputIt;

template<class Container, class CharT, class Traits, class Delimiter>
auto str_split_to(std::basic_string_view<CharT, Traits> str, Delimiter&& delim) -> Container;

template<template<class...> class Container, class CharT, class Traits, class Delimiter>
auto str_split_to(std::basic_string_view<CharT, Traits> st, Delimiter&& delim) -> Container<std::basic_string_view<CharT, Traits>>;

template<class CharT, class Traits, class Delimiter>
auto str_split(std::basic_string_view<CharT, Traits> str, Delimiter&& delim) -> std::vector<std::basic_string_view<CharT, Traits>>;

aryann · 2026-05-11T16:42:57Z

Here's how it could look without the implicit conversion operators. This is modeled after std::format_to and std::ranges::to.

template<class OutputIt, class CharT, class Traits, class Delimiter>
auto str_split_to(OutputIt to, std::basic_string_view<CharT, Traits> str, Delimiter&& delim) -> OutputIt;

template<class Container, class CharT, class Traits, class Delimiter>
auto str_split_to(std::basic_string_view<CharT, Traits> str, Delimiter&& delim) -> Container;

template<template<class...> class Container, class CharT, class Traits, class Delimiter>
auto str_split_to(std::basic_string_view<CharT, Traits> st, Delimiter&& delim) -> Container<std::basic_string_view<CharT, Traits>>;

template<class CharT, class Traits, class Delimiter>
auto str_split(std::basic_string_view<CharT, Traits> str, Delimiter&& delim) -> std::vector<std::basic_string_view<CharT, Traits>>;

I replaced the initial functions with these and added some basic tests + an example.

One downside is that you can only pass in basic_string_view. Type deduction fails with const char* and std::string. A non-exhaustive list of options (which we can explore in another PR):

Add explicit overloads for std::basic_string<CharT, Traits, Alloc> and const CharT*.
Make the input more generic and use concepts to ensure it's a range.

The purpose of this change is to start the conversation on the best interface for this library.

…modules

aryann requested a review from bartholomaios May 10, 2026 00:44

bartholomaios reviewed May 10, 2026

View reviewed changes

aryann force-pushed the scaffolding branch from 3bf4388 to c55f548 Compare May 11, 2026 15:35

aryann added 8 commits May 11, 2026 09:49

Adds an MVP set of functions

e6632f4

The purpose of this change is to start the conversation on the best interface for this library.

Adds more details to the concepts that govern the range constructor

b25dc17

Adds missing newline

1d96659

Renames some of the files and fixes the includes

6a286de

Replaces the split function with str_split_to

557e286

Adds split_by_char

1475000

Adds tests for different char types

ed47b98

Removes some TODOs

0bfcecf

aryann force-pushed the scaffolding branch from dd0ab33 to 0bfcecf Compare May 11, 2026 16:49

aryann added 5 commits May 11, 2026 09:58

Adds missing include

e96c545

Adds support for importing std types via modules

bb60de3

An attempt to fix redefinition errors in one of the gcc builds

4223276

Use textual includes for the test target since it's not setup to use …

236502b

…modules

Fixes a typo and adds a missing return

0d0cdee


		#include <string>
		#include <string_view>

Conversation

aryann commented May 10, 2026

Uh oh!

coveralls commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 25686618867

Coverage: 100.0%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

bartholomaios left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bartholomaios commented May 10, 2026

Uh oh!

aryann commented May 10, 2026

Uh oh!

bartholomaios commented May 11, 2026

Uh oh!

aryann commented May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coveralls commented May 10, 2026 •

edited

Loading