feat: Add unset argument to git_config_set() and git_config_global_set() - #273
Conversation
| #' expression `value`, equivalent `git config --unset`. If `TRUE` and | ||
| #' `value == NULL`, delete all entries for `name`, equivalent to | ||
| #' `git config --unset-all`. | ||
| git_config_set <- function( |
There was a problem hiding this comment.
why an argument rather than another function? I was thinking that git_config_set(unset = TRUE) sounds weird.
There was a problem hiding this comment.
A bit of an oxymoron for sure. I can think of two other interfaces that might be more intuitive:
- Adding two new functions,
git_config_unset()andgit_config_global_unset() - Instead of an
unsetargument, add an argument named, say,replacewhich takes a regular expression. Existing values matchingreplacewould be replaced with the newvalue, which could be set toNULLto remove those existing values. Then instead ofgit_config_set(value = "ccc", unset = TRUE), one could usegit_config_set(value = NULL, replace = "ccc"), which seems more clear.
Personally, I like option 2 better, since it uses the same syntax for deleting config var values as before (i.e., value = NULL; it's more similar to the libgit2 API; and there are fewer functions to keep track of. I'd be happy to amend the PR to take whichever approach you think is best, and address your other comments then too. Thank you for taking the time to review!
There was a problem hiding this comment.
@iofarm let's go with option 1 if that's ok with you. Not so close to Git, but maybe more intuitive?
Adding two new functions, git_config_unset() and git_config_global_unset()
There was a problem hiding this comment.
Great, I've added commits to implement option 1
…in git_config_unset()
|
Thanks you for the contribution. This implementation is too complex for my taste. Most users don't know what a regex/engine is and just want to unset a single value. Advanced users that do want to unset specific values based on a regex, probably understand that they should use portable regex, not something engine specific. It is out of scope for us to get into technical details of this. I'll simplify this PR a bit to the essence. |
b0168a2 to
1c05177
Compare
Proposal to resolve #264 by adding an
unsetargument togit_config_set()andgit_config_global_set(). This would allow removing specific values of multivalued config variables. In conjunction with #261 (merged), this allows full manipulation of multivalued config variables.Example usage is demonstrated in test-config.R ("multivar, local, custom config roundtrip"). A different interface for unsetting values might be more intuitive - I'm open to suggestions.
In addition: Because libgit2 uses regular expressions to match config values to delete, and it may be linked against different regex engines on different machines, this PR also patches
libgit2_config()to report the regex engine in$regex_backend. Becausegit_libgit2_feature_backend()was only added in libgit2 1.9, this is reported as"unknown"if the libgit2 version is older.