Force-pushed with changes as suggested by MarcoFalke (#22953 (comment)).

Pinging @practicalswift as someone who has always shown interest in potential boost replacements and string processing helpers, with lots of contributions in that area. Maybe you feel like taking a look :)

I would still add a unit test even though there are tests for the internal helper. Maybe it's a little bit over the top so feel free to ignore my suggestion.

(Something like this possibly: kiminuo@ac6dd5a)

Any reason the tests aren't fixed up as well?

src/test/fuzz/script_assets_test_minimizer.cpp:    boost::algorithm::split(words, str, boost::algorithm::is_any_of(","));

Also, if you add a unit test, might as extend the string fuzz target.

There non-tests occurrences too:

src\rpc\server.cpp:
  410          std::vector<std::string> vargNames;
  411:         boost::algorithm::split(vargNames, argNamePattern, boost::algorithm::is_any_of("|"));
  412          auto fr = argsIn.end();

src\test\transaction_tests.cpp:
  72      std::vector<std::string> words;
  73:     boost::algorithm::split(words, strFlags, boost::algorithm::is_any_of(","));
  74  

src\test\fuzz\script_assets_test_minimizer.cpp:
  133      std::vector<std::string> words;
  134:     boost::algorithm::split(words, str, boost::algorithm::is_any_of(","));
  135  

If I'm not missing something.

edit: Ah, I'm not aware what the difference is between boost::algorithm::split and boost::split. Seems like none?

I would still add a unit test even though there are tests for the internal helper. Maybe it's a little bit over the top so feel free to ignore my suggestion.

I agree that it's a good idea to add a unit test.

(Something like this possibly: kiminuo@ac6dd5a)

Thanks! As quick feedback, can you change the commit subject to be more verbose (e.g. "test: add unit tests for SplitString helper") and put the test into src/test/util_tests.cpp, instead of creating a new file? I'm happy to either add your commit to this PR, or review your PR if you decide to open your own, based on this one.

Any reason the tests aren't fixed up as well?
...

There non-tests occurrences too:
...

Oh, I only greped for boost::split, not being aware that there is also boost::algorithm::split, which seems to be a synonym. Force-pushed with changes to tackle also the remaining occurences. Also found that there were some unnecessary boost includes in src/torcontrol.cpp that I forgot to remove in the previous push.

(Something like this possibly: kiminuo@ac6dd5a)

Thanks! As quick feedback, can you change the commit subject to be more verbose (e.g. "test: add unit tests for SplitString helper") and put the test into src/test/util_tests.cpp, instead of creating a new file? I'm happy to either add your commit to this PR, or review your PR if you decide to open your own, based on this one.

Modified here: kiminuo@4516727. Anyway, feel free to modify the commit in whatever way you like :)

The fuzz test I mentioned: (might not compile)

diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp
index 0c1b45b86c..e5a09c0013 100644
--- a/src/test/fuzz/string.cpp
+++ b/src/test/fuzz/string.cpp
@@ -127,6 +127,7 @@ FUZZ_TARGET(string)
         int64_t amount_out;
         (void)ParseFixedPoint(random_string_1, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 1024), &amount_out);
     }
+    (void)SplitString(random_string_1, fuzzed_data_provider.ConsumeIntegral<char>());
     {
         (void)Untranslated(random_string_1);
         const bilingual_str bs1{random_string_1, random_string_2};

The fuzz test I mentioned: (might not compile)

Oh, I didn't see that in your previous comment. It compiles, I added another commit 👌

Concept ACK - this is low-overhead for dropping more Boost dependence. Surprised we can't also nuke anything from test/lint/lint-includes.sh

Concept ACK. I don't think this boost use is very bad (it's a header-only library) but if we can replace it with functionality we already have internally, who not.

Concept ACK - this is low-overhead for dropping more Boost dependence. Surprised we can't also nuke anything from test/lint/lint-includes.sh

Well, split is still used in some places (httprpc.cpp for example), this only replaces the single-separator case. It will take some more work to get rid of all boost/algorithm/string.hpp use, if that's worthwhile.

Force-pushed the alternative variant as suggested by @martinus, changing spanparsing::Split to a templated function and defining an alias SplitString as suggested by @MarcoFalke (see #22953 (comment) and #22953 (comment)).

Windows build error seems unrelated, in wallet_address_types.py:

OSError: [WinError 10048] Only one usage of each socket address (protocol/network address/port) is normally permitted

Rebased on master.

Rebased on master again (there was a conflict in src/rest.cpp after #24098 has been merged).

@martinus want to take another look here?

Code review ACK a62e844. Ran all tests. I also like that with boost::split it was not obvious that the resulting container was cleared, and with SplitString API that's obvious.

After that PR there is only one usage of boost::split left in core_read.cpp, that would be a nice followup PR 🙂