-
Notifications
You must be signed in to change notification settings - Fork 613
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.
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
[wpiutil C++] Add remove_prefix() and remove_suffix() #6118
[wpiutil C++] Add remove_prefix() and remove_suffix() #6118
Conversation
StringExtras.h is considered a generated file because it used to be updated by the update_llvm.py script. It's now in limbo because it's an LLVM file we manually modify. Whether we format it depends on whether we plan on updating pieces from upstream again. Idk. |
Most of the usages are of the following idiom: if str.startswith(literal):
str = remove_prefix(str, literal) Two questions:
|
We might, but I personally am fine with relying on the review process checking the literals are identical (especially compared to checking the length matches that of the literal), which generally isn't that bad since usually they're within a few lines of each other. I also think it'd be annoying to have to pick a spot to declare the variable with the value of the literal, but I get the concerns over keeping both spots in sync with each other.
The ones with an explicit call-site check usually do other logic, so you need the explicit check anyways. We could maybe add a parameter to prevent |
I'm trying a change (d0de6af) to fix the MacOS build error, but it probably should get moved into a separate PR once it's verified to work. |
(Note for myself so I don't forget)
|
Local patch to fix compilation that should not be on main
Also fix a spot that erronerously translated drop_back to remove_prefix
I'm not the happiest with a few of the translations- I think Peter had a really good vision for the most common use-case, but some of the odder occurrences were difficult to do cleanly. (Specifically, the |
Use value_or instead of value (to avoid throwing) Dereference optionals Undo accidental change to NetworkTablesModel::Update() behavior schemas of arrays
Notes:
drop_front(str, prefix.size())
didn't previously check that the value began with the prefix, so this might change the behavior of those spots. (There's probably other ways that the property is guaranteed, like with the listener prefixes, but I didn't completely verify that)remove_suffix()
doesn't change a string not ending with the suffix. I'm not sure if the code is the clearest, though- I may revert this. (Please give feedback on this)NetworkTablesProvider
, I made my best guess on where the 6 came from based on the rest of the function, but I'd like to make sure someone double-checks that it makes sense.