From 12a4883feb53d08a80b3c049415105b0543f27a5 Mon Sep 17 00:00:00 2001 From: Sergey Organov Date: Sun, 3 Mar 2024 12:50:32 +0300 Subject: [PATCH] clean: improve -n and -f implementation and documentation What -n actually does in addition to its documented behavior is ignoring of configuration variable clean.requireForce, that makes sense provided -n prevents files removal anyway. So, first, document this in the manual, and then modify implementation to make this more explicit in the code. Improved implementation also stops to share single internal variable 'force' between command-line -f option and configuration variable clean.requireForce, resulting in more clear logic. Two error messages with slightly different text depending on if clean.requireForce was explicitly set or not, are merged into a single one. The resulting error message now does not mention -n as well, as it neither matches intended clean.requireForce usage nor reflects clarified implementation. Documentation of clean.requireForce is changed accordingly. Signed-off-by: Sergey Organov Signed-off-by: Junio C Hamano --- Documentation/config/clean.txt | 4 ++-- Documentation/git-clean.txt | 2 ++ builtin/clean.c | 23 +++++++++-------------- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/Documentation/config/clean.txt b/Documentation/config/clean.txt index f05b9403b5ad90..b19ca210f39b57 100644 --- a/Documentation/config/clean.txt +++ b/Documentation/config/clean.txt @@ -1,3 +1,3 @@ clean.requireForce:: - A boolean to make git-clean do nothing unless given -f, - -i, or -n. Defaults to true. + A boolean to make git-clean refuse to delete files unless -f + or -i is given. Defaults to true. diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt index 69331e3f05a13e..662eebb852076a 100644 --- a/Documentation/git-clean.txt +++ b/Documentation/git-clean.txt @@ -49,6 +49,8 @@ OPTIONS -n:: --dry-run:: Don't actually remove anything, just show what would be done. + Configuration variable clean.requireForce is ignored, as + nothing will be deleted anyway. -q:: --quiet:: diff --git a/builtin/clean.c b/builtin/clean.c index d90766cad3a0ba..41502dcb0ddeed 100644 --- a/builtin/clean.c +++ b/builtin/clean.c @@ -25,7 +25,7 @@ #include "help.h" #include "prompt.h" -static int force = -1; /* unset */ +static int require_force = -1; /* unset */ static int interactive; static struct string_list del_list = STRING_LIST_INIT_DUP; static unsigned int colopts; @@ -128,7 +128,7 @@ static int git_clean_config(const char *var, const char *value, } if (!strcmp(var, "clean.requireforce")) { - force = !git_config_bool(var, value); + require_force = git_config_bool(var, value); return 0; } @@ -920,7 +920,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) { int i, res; int dry_run = 0, remove_directories = 0, quiet = 0, ignored = 0; - int ignored_only = 0, config_set = 0, errors = 0, gone = 1; + int ignored_only = 0, force = 0, errors = 0, gone = 1; int rm_flags = REMOVE_DIR_KEEP_NESTED_GIT; struct strbuf abs_path = STRBUF_INIT; struct dir_struct dir = DIR_INIT; @@ -946,22 +946,17 @@ int cmd_clean(int argc, const char **argv, const char *prefix) }; git_config(git_clean_config, NULL); - if (force < 0) - force = 0; - else - config_set = 1; argc = parse_options(argc, argv, prefix, options, builtin_clean_usage, 0); - if (!interactive && !dry_run && !force) { - if (config_set) - die(_("clean.requireForce set to true and neither -i, -n, nor -f given; " - "refusing to clean")); - else - die(_("clean.requireForce defaults to true and neither -i, -n, nor -f given;" + /* Dry run won't remove anything, so requiring force makes no sense */ + if (dry_run) + require_force = 0; + + if (require_force != 0 && !force && !interactive) + die(_("clean.requireForce is true and neither -f nor -i given:" " refusing to clean")); - } if (force > 1) rm_flags = 0;