[vcpkg] Rewriting CmdLineBuilder/Command (3/n) #15673
Conversation
strega-nil
force-pushed
the
fix-cmdlinebuilder
branch
from
d426bef
to
383050f
Compare
strega-nil
changed the title
strega-nil
force-pushed
the
fix-cmdlinebuilder
branch
from
383050f
to
fbf777c
Compare
| explicit Command(const std::string& s) { string_arg(s); } | ||
| explicit Command(const char* s) { string_arg({s, ::strlen(s)}); } | ||
|
|
||
| Command& path_arg(const fs::path& p) & { return string_arg(fs::u8string(p)); } |
There was a problem hiding this comment.
I'm a bit unhappy with the combination of the "data" name plus "data" named functions; specifically, it is not clear from the name of the type nor the name of the functions that these are mutating functions -- you need to either notice that the return type is & or read the implementation to know that these will mutate the existing object.
This is why I named the original type "Builder": so that the functions like path_arg could be short instead of append_path_arg() yet there was still nomenclature to note that these functions are mutating.
There was a problem hiding this comment.
I also generally don't see sufficient motivation for change in the name, this just introduces code churn for insufficient gain IMO.
There was a problem hiding this comment.
I disagree; it's fundamentally no longer a "builder", it's "the thing itself".
Also, I would argue that this kind of naming for functions makes sense; see Rust's Command type, which does the same thing.
strega-nil
changed the title
Rename CmdLineBuilder to Command, since it's no longer a builder but an actual data type
strega-nil
force-pushed
the
fix-cmdlinebuilder
branch
from
fbf777c
to
4359ab7
Compare
Rename CmdLineBuilder to Command, since it's no longer a builder but an actual data type
Rename CmdLineBuilder to Command, since it's no longer a builder but an actual data type.
This is the 3rd PR in the "rewrite CmdLineBuilder" saga; at this point, we need a new type name :)