serenity
serenity copied to clipboard
man: Remove DeprecatedString
Related to #17128
This pull request requires the minimum set of patches that I could think of to remove DeprecatedString from the man utility.
This includes:
- Adding
Stringvariants toadd_optionandadd_positional_argumentin theArgsParser - Converting
render_to_terminalfromDeprecatedStringtoErrorOr<String>(which meant also changing themdutility) - Removing all of the
DeprecatedStringfromman
When converting render_to_terminal to ErrorOr<String> I made a conscious decision to ignore the other methods on the class, even if they would have been simple to convert, in order to keep the patch as small as possible.
As discussed on the Discord another possible improvement here would be to have a version of Core::System::posix_spawnp that can take StringViews instead of const char*, but I think that is something best dealt with in another pull request.
@LucasChollet Thanks for the review :)
I have pushed changes based on your comments
Side note: Looks like we need a new variant of
Core::System::posix_spawnp, because copying a StringView just to add a NUL byte is a bit silly. Currently it's necessary, yes, so let's merge this. (After the String-ArgsParser thing is done.)
Completely agree. This was something that was also discussed on the Discord