[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    [PATCH] clean: improve -n and -f implementation and documentation
From:       Sergey Organov <sorganov () gmail ! com>
Date:       2024-02-29 19:07:18
Message-ID: 875xy76qe1.fsf () osv ! gnss ! ru
[Download RAW message or body]

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.

The error messages now do not mention -n as well, as it seems
unnecessary and does not reflect clarified implementation.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/git-clean.txt |  2 ++
 builtin/clean.c             | 26 +++++++++++++-------------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 69331e3f05a1..662eebb85207 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 d90766cad3a0..fcc50d08ee9b 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,21 +946,21 @@ 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; "
+	/* Dry run won't remove anything, so requiring force makes no sense */
+	if(dry_run)
+		require_force = 0;
+
+	if (!force && !interactive) {
+		if (require_force > 0)
+			die(_("clean.requireForce set to true and neither -f, nor -i given; "
+				  "refusing to clean"));
+		else if (require_force < 0)
+			die(_("clean.requireForce defaults to true and neither -f, nor -i given; "
 				  "refusing to clean"));
-		else
-			die(_("clean.requireForce defaults to true and neither -i, -n, nor -f given;"
-				  " refusing to clean"));
 	}
 
 	if (force > 1)

base-commit: 0f9d4d28b7e6021b7e6db192b7bf47bd3a0d0d1d
-- 
2.25.1


[prev in list] [next in list] [prev in thread] [next in thread]