diff mbox series

[v3,2/2] worktree: rename is_worktree_locked to worktree_lock_reason

Message ID 20181030062409.42169-2-nbelakovski@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v3,1/2] worktree: update documentation for lock_reason and lock_reason_valid | expand

Commit Message

Nickolai Belakovski Oct. 30, 2018, 6:24 a.m. UTC 
From: Nickolai Belakovski <nbelakovski@gmail.com>

A function prefixed with 'is_' would be expected to return a boolean,
however this function returns a string.

Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com>
---
 builtin/worktree.c | 10 +++++-----
 worktree.c         |  2 +-
 worktree.h         |  4 ++--
 3 files changed, 8 insertions(+), 8 deletions(-)

Comments

Junio C Hamano Oct. 31, 2018, 2:41 a.m. UTC | #1 
nbelakovski@gmail.com writes:

> From: Nickolai Belakovski <nbelakovski@gmail.com>
>
> A function prefixed with 'is_' would be expected to return a boolean,
> however this function returns a string.
>
> Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com>
> ---

Given that there is a clear documentation in worktree.h, and a
pointer that is not NULL is true in "if/while(ptr)", I'd say this
change is a borderline "meh".

I'll queue this on top of 1/2 and try merging to the integration
topics to see if it interacts with any other topics in flight.
Since the patch has already been written, let's not waste the effort
if there is no conflict with anybody else (otherwise I'd discard
this one---a "meh" patch is not worth having to worry about conflict
resolution).

Thanks.

>  builtin/worktree.c | 10 +++++-----
>  worktree.c         |  2 +-
>  worktree.h         |  4 ++--
>  3 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index c4abbde2b..5e8402617 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -245,7 +245,7 @@ static void validate_worktree_add(const char *path, const struct add_opts *opts)
>  	if (!wt)
>  		goto done;
>  
> -	locked = !!is_worktree_locked(wt);
> +	locked = !!worktree_lock_reason(wt);
>  	if ((!locked && opts->force) || (locked && opts->force > 1)) {
>  		if (delete_git_dir(wt->id))
>  		    die(_("unable to re-add worktree '%s'"), path);
> @@ -682,7 +682,7 @@ static int lock_worktree(int ac, const char **av, const char *prefix)
>  	if (is_main_worktree(wt))
>  		die(_("The main working tree cannot be locked or unlocked"));
>  
> -	old_reason = is_worktree_locked(wt);
> +	old_reason = worktree_lock_reason(wt);
>  	if (old_reason) {
>  		if (*old_reason)
>  			die(_("'%s' is already locked, reason: %s"),
> @@ -714,7 +714,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
>  		die(_("'%s' is not a working tree"), av[0]);
>  	if (is_main_worktree(wt))
>  		die(_("The main working tree cannot be locked or unlocked"));
> -	if (!is_worktree_locked(wt))
> +	if (!worktree_lock_reason(wt))
>  		die(_("'%s' is not locked"), av[0]);
>  	ret = unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id));
>  	free_worktrees(worktrees);
> @@ -787,7 +787,7 @@ static int move_worktree(int ac, const char **av, const char *prefix)
>  	validate_no_submodules(wt);
>  
>  	if (force < 2)
> -		reason = is_worktree_locked(wt);
> +		reason = worktree_lock_reason(wt);
>  	if (reason) {
>  		if (*reason)
>  			die(_("cannot move a locked working tree, lock reason: %s\nuse 'move -f -f' to override or unlock first"),
> @@ -900,7 +900,7 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
>  	if (is_main_worktree(wt))
>  		die(_("'%s' is a main working tree"), av[0]);
>  	if (force < 2)
> -		reason = is_worktree_locked(wt);
> +		reason = worktree_lock_reason(wt);
>  	if (reason) {
>  		if (*reason)
>  			die(_("cannot remove a locked working tree, lock reason: %s\nuse 'remove -f -f' to override or unlock first"),
> diff --git a/worktree.c b/worktree.c
> index b0d0b5426..befdbe7fa 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -235,7 +235,7 @@ int is_main_worktree(const struct worktree *wt)
>  	return !wt->id;
>  }
>  
> -const char *is_worktree_locked(struct worktree *wt)
> +const char *worktree_lock_reason(struct worktree *wt)
>  {
>  	assert(!is_main_worktree(wt));
>  
> diff --git a/worktree.h b/worktree.h
> index 6b12a3cf6..55d449b6a 100644
> --- a/worktree.h
> +++ b/worktree.h
> @@ -10,7 +10,7 @@ struct worktree {
>  	char *path;
>  	char *id;
>  	char *head_ref;		/* NULL if HEAD is broken or detached */
> -	char *lock_reason;	/* private - use is_worktree_locked */
> +	char *lock_reason;	/* private - use worktree_lock_reason */
>  	struct object_id head_oid;
>  	int is_detached;
>  	int is_bare;
> @@ -60,7 +60,7 @@ extern int is_main_worktree(const struct worktree *wt);
>   * Return the reason string if the given worktree is locked or NULL
>   * otherwise.
>   */
> -extern const char *is_worktree_locked(struct worktree *wt);
> +extern const char *worktree_lock_reason(struct worktree *wt);
>  
>  #define WT_VALIDATE_WORKTREE_MISSING_OK (1 << 0)
diff mbox series

Patch

diff --git a/builtin/worktree.c b/builtin/worktree.c
index c4abbde2b..5e8402617 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -245,7 +245,7 @@  static void validate_worktree_add(const char *path, const struct add_opts *opts)
 	if (!wt)
 		goto done;
 
-	locked = !!is_worktree_locked(wt);
+	locked = !!worktree_lock_reason(wt);
 	if ((!locked && opts->force) || (locked && opts->force > 1)) {
 		if (delete_git_dir(wt->id))
 		    die(_("unable to re-add worktree '%s'"), path);
@@ -682,7 +682,7 @@  static int lock_worktree(int ac, const char **av, const char *prefix)
 	if (is_main_worktree(wt))
 		die(_("The main working tree cannot be locked or unlocked"));
 
-	old_reason = is_worktree_locked(wt);
+	old_reason = worktree_lock_reason(wt);
 	if (old_reason) {
 		if (*old_reason)
 			die(_("'%s' is already locked, reason: %s"),
@@ -714,7 +714,7 @@  static int unlock_worktree(int ac, const char **av, const char *prefix)
 		die(_("'%s' is not a working tree"), av[0]);
 	if (is_main_worktree(wt))
 		die(_("The main working tree cannot be locked or unlocked"));
-	if (!is_worktree_locked(wt))
+	if (!worktree_lock_reason(wt))
 		die(_("'%s' is not locked"), av[0]);
 	ret = unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id));
 	free_worktrees(worktrees);
@@ -787,7 +787,7 @@  static int move_worktree(int ac, const char **av, const char *prefix)
 	validate_no_submodules(wt);
 
 	if (force < 2)
-		reason = is_worktree_locked(wt);
+		reason = worktree_lock_reason(wt);
 	if (reason) {
 		if (*reason)
 			die(_("cannot move a locked working tree, lock reason: %s\nuse 'move -f -f' to override or unlock first"),
@@ -900,7 +900,7 @@  static int remove_worktree(int ac, const char **av, const char *prefix)
 	if (is_main_worktree(wt))
 		die(_("'%s' is a main working tree"), av[0]);
 	if (force < 2)
-		reason = is_worktree_locked(wt);
+		reason = worktree_lock_reason(wt);
 	if (reason) {
 		if (*reason)
 			die(_("cannot remove a locked working tree, lock reason: %s\nuse 'remove -f -f' to override or unlock first"),
diff --git a/worktree.c b/worktree.c
index b0d0b5426..befdbe7fa 100644
--- a/worktree.c
+++ b/worktree.c
@@ -235,7 +235,7 @@  int is_main_worktree(const struct worktree *wt)
 	return !wt->id;
 }
 
-const char *is_worktree_locked(struct worktree *wt)
+const char *worktree_lock_reason(struct worktree *wt)
 {
 	assert(!is_main_worktree(wt));
 
diff --git a/worktree.h b/worktree.h
index 6b12a3cf6..55d449b6a 100644
--- a/worktree.h
+++ b/worktree.h
@@ -10,7 +10,7 @@  struct worktree {
 	char *path;
 	char *id;
 	char *head_ref;		/* NULL if HEAD is broken or detached */
-	char *lock_reason;	/* private - use is_worktree_locked */
+	char *lock_reason;	/* private - use worktree_lock_reason */
 	struct object_id head_oid;
 	int is_detached;
 	int is_bare;
@@ -60,7 +60,7 @@  extern int is_main_worktree(const struct worktree *wt);
  * Return the reason string if the given worktree is locked or NULL
  * otherwise.
  */
-extern const char *is_worktree_locked(struct worktree *wt);
+extern const char *worktree_lock_reason(struct worktree *wt);
 
 #define WT_VALIDATE_WORKTREE_MISSING_OK (1 << 0)