Skip to content

git: Improve SafeCmd interface

Patrick Steinhardt requested to merge pks-git-safecmd-refactorings into master

While working on a way to make config options globally available, I decided to just use the ConfigPair option everywhere to pass config keys. The benefit of this would've been that eventually, we can easily switch to use environment variables at a later point as soon as it is implemented upstream. It took me some time to actually realize that it's not possible to use it as a global variable, as it was only intended for use by git config. This led me down the rabbit hole, culminating in this MR refactoring some of the SafeCmd interface. Most importantly, it:

  • Disambiguates command-specific Options and GlobalOptions such that it's not possible to accidentally pass something like ConfigPair as a global option if it doesn't support that. I would've liked to also rename Options to CommandFlags or something like this to make the difference clearer, but there's already enough global changes right now. It's also possible to now render differently based on whether it's used as global or command option, which is especially helpful in the context of ContextPair.
  • It converts SubSubCmd from an Option to a Cmd. It's weird that it's been an Option previously, as you had to pass it along with all the other typical options. Now, it can simply be used instead of a SubCmd type with an additional verb, e.g. "set-url" for git-remote.
  • I've touched up the interfaces to remove those IsOption and IsCmd functions. These really only had the purpose to disambiguate interfaces, as they all implemented the same ValidateArguments() function. This was a bit awkward, so I've just decided to use separate functions OptionArgs(), GlobalOptionArgs() and CommandArgs() which is much clearer in my opinion and doesn't require those useless helper funcitons.
  • I've removed the CmdStream type from the external interface. We should just use git.WithStdout() and the likes nowadays.
Edited by Patrick Steinhardt

Merge request reports

Loading