refactor: consolidate string utilities on sstr and remove legacy SStr#325
refactor: consolidate string utilities on sstr and remove legacy SStr#325plexoos wants to merge 3 commits into
Conversation
This branch removes the parallel `SStr`/`sstr` string utility split and standardizes the codebase on `sstr` plus straightforward standard C++ helpers where custom wrappers were no longer needed. The goal is to reduce duplication, shrink the maintenance surface, and make string/path/env handling easier to follow and evolve. `SStr` and `sstr` had overlapping responsibilities, which made it unclear which API should be preferred and kept legacy patterns alive longer than necessary. This refactor migrates the remaining `SStr` call sites to `sstr`, `spath`, `ssys`, and simple `std::string`/stream-based code, then removes the obsolete `SStr` implementation, tests, and build references. The result is a smaller and more consistent utility layer with no intended functional change.
Cpp-Linter Report
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97e32aad93
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| char delim = ',' ; | ||
| std::vector<int> ivec ; | ||
| SStr::ISplit( spec, ivec, delim ); | ||
| sstr::split<int>( ivec, spec, delim ); |
There was a problem hiding this comment.
Switching from SStr::ISplit to sstr::split<int> changes behavior for malformed specs like "1,,3" or a trailing comma: sstr::split does iss >> v and pushes v even when extraction fails, so an empty token can append an indeterminate integer. In this constructor that can trigger the bounds assert or enable an unintended bit unpredictably; previously ISplit used atoi, which produced a deterministic 0 for empty tokens.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR standardizes the codebase on sstr/spath/ssys (and basic std::string utilities) by migrating remaining SStr call sites and removing the legacy SStr implementation and its tests/build references, reducing duplicated string utility APIs.
Changes:
- Migrates formatting/splitting/trimming/concat usage from
SStrtosstr/spath/ssys(plus small local helpers where needed). - Removes
sysrap/SStr.*and the associatedSStrTestplus build/script references. - Extends
sysrap/sstr.hwithSimpleMatchand minor modernizations (std::string_view).
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| u4/U4Material.cc | Replaces SStr formatting/splitting with sstr and standard std::string helpers. |
| u4/U4.cc | Drops legacy SStr.hh include as part of consolidation. |
| u4/tests/U4MaterialPropertyVectorTest.cc | Removes unused SStr.hh include. |
| sysrap/tests/SStrTest.cc | Removes legacy SStr unit test. |
| sysrap/tests/SPathTest.cc | Switches legacy SStr usage to spath helpers in tests. |
| sysrap/tests/SEvtTest.cc | Uses spath::Name instead of SStr::Name in test output naming. |
| sysrap/tests/SEvt__LoadTest.sh | Removes SStr.cc from the standalone test build list. |
| sysrap/tests/CMakeLists.txt | Removes SStrTest.cc from the test sources list. |
| sysrap/SVec.cc | Replaces SStr::ReplaceChars dependency with a local std::string-based implementation. |
| sysrap/SSys.cc | Replaces SStr::Concat with std::string composition for python -c execution. |
| sysrap/SStr.hh | Removes legacy SStr header. |
| sysrap/sstr.h | Adds SimpleMatch and modernizes internals (e.g. std::string_view). |
| sysrap/SStr.cc | Removes legacy SStr implementation. |
| sysrap/SSim.cc | Migrates trimming/splitting from SStr to sstr. |
| sysrap/SPPM.cc | Migrates SStr string predicates to sstr. |
| sysrap/SPath.cc | Removes SStr dependency by inlining equivalent formatting helpers. |
| sysrap/SOpticksResource.cc | Migrates split/format calls from SStr to sstr. |
| sysrap/SOpticksKey.cc | Migrates concat/split calls from SStr to sstr. |
| sysrap/SOpticks.cc | Switches legacy save/concat usage to spath + std::string. |
| sysrap/SEnabled.cc | Replaces SStr::ISplit with sstr::split. |
| sysrap/CMakeLists.txt | Drops SStr sources/headers from the SysRap target. |
| qudarap/tests/QCKTest.cc | Removes legacy SStr.hh include. |
| CSGOptiX/Frame.cc | Uses ssys env helpers instead of SStr::GetEValue. |
| CSGOptiX/CSGOptiX.cc | Migrates concat/contains/replace-end usage from SStr to sstr. |
| CSG/tests/DemoGrid.cc | Replaces SStr env/grid parsing helpers with ssys/sstr and local parsing. |
| CSG/tests/DemoGeo.cc | Replaces SStr parsing/prefix checks with sstr + local parsing. |
| CSG/tests/CSGTargetTest.cc | Migrates split calls from SStr to sstr. |
| CSG/tests/CSGTargetGlobalTest.cc | Removes legacy SStr.hh include. |
| CSG/tests/CSGNodeScanTest.cc | Replaces SStr::ISplit with a local parser using sstr::split. |
| CSG/tests/CSGFoundry_getCenterExtent_Test.cc | Migrates split calls from SStr to sstr. |
| CSG/CSGFoundry.cc | Migrates split + simple-match usage from SStr to sstr and updates some formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…Path.cc Agent-Logs-Url: https://github.com/BNLNPPS/simphony/sessions/542d7099-2d3b-499c-8dec-dc832766a3cb Co-authored-by: plexoos <5005079+plexoos@users.noreply.github.com>
This branch removes the parallel
SStr/sstrstring utility split and standardizes the codebase onsstrplus straightforward standard C++ helpers where custom wrappers were no longer needed.The goal is to reduce duplication, shrink the maintenance surface, and make string/path/env handling
easier to follow and evolve.
SStrandsstrhad overlapping responsibilities, which made itunclear which API should be preferred and kept legacy patterns alive longer than necessary.
This refactor migrates the remaining
SStrcall sites tosstr,spath,ssys, and simplestd::string/stream-based code, then removes the obsoleteSStrimplementation, tests, and buildreferences. The result is a smaller and more consistent utility layer with no intended functional
change.