[v4,27/28] trailer_set_*(): put out parameter at the end

Commit Message

Linus Arver Feb. 6, 2024, 5:12 a.m. UTC

From: Linus Arver <[e-mail]>

The new trailer_config_set_*() functions which were introduced a few
patches ago put the out parameter (the variable being mutated) at the
end of the parameter list.

Put the out parameter at the end for these functions for these existing
trailer_set_*() functions for consistency. This also avoids confusion
for API users because otherwise these two sets of functions look rather
similar in <trailer.h> even though they have completely different out
parameters.

Signed-off-by: Linus Arver <[e-mail]>
---
 builtin/interpret-trailers.c |  6 +++---
 trailer.c                    | 24 ++++++++++++------------
 trailer.h                    |  6 +++---
 3 files changed, 18 insertions(+), 18 deletions(-)

Comments

Christian Couder Feb. 12, 2024, 11:39 p.m. UTC | #1

On Tue, Feb 6, 2024 at 6:12 AM Linus Arver via GitGitGadget
<[e-mail]> wrote:
>
> From: Linus Arver <[e-mail]>
>
> The new trailer_config_set_*() functions which were introduced a few
> patches ago put the out parameter (the variable being mutated) at the
> end of the parameter list.
>
> Put the out parameter at the end for these functions for these existing

s/for these functions for these/for the/


> trailer_set_*() functions for consistency. This also avoids confusion
> for API users because otherwise these two sets of functions look rather
> similar in <trailer.h> even though they have completely different out
> parameters.

Linus Arver Feb. 13, 2024, 6:14 p.m. UTC | #2

Christian Couder <[e-mail]> writes:

> On Tue, Feb 6, 2024 at 6:12 AM Linus Arver via GitGitGadget
> <[e-mail]> wrote:
>>
>> From: Linus Arver <[e-mail]>
>>
>> The new trailer_config_set_*() functions which were introduced a few
>> patches ago put the out parameter (the variable being mutated) at the
>> end of the parameter list.
>>
>> Put the out parameter at the end for these functions for these existing
>
> s/for these functions for these/for the/

That wording is much better; will update.

>> trailer_set_*() functions for consistency. This also avoids confusion
>> for API users because otherwise these two sets of functions look rather
>> similar in <trailer.h> even though they have completely different out
>> parameters.

Patch

diff --git a/builtin/interpret-trailers.c b/builtin/interpret-trailers.c
index 9657b0d067c..d0c09d1d73b 100644
--- a/builtin/interpret-trailers.c
+++ b/builtin/interpret-trailers.c
@@ -28,21 +28,21 @@  static int option_parse_where(const struct option *opt,
 			      const char *arg, int unset UNUSED)
 {
 	/* unset implies NULL arg, which is handled in our helper */
-	return trailer_set_where(opt->value, arg);
+	return trailer_set_where(arg, opt->value);
 }
 
 static int option_parse_if_exists(const struct option *opt,
 				  const char *arg, int unset UNUSED)
 {
 	/* unset implies NULL arg, which is handled in our helper */
-	return trailer_set_if_exists(opt->value, arg);
+	return trailer_set_if_exists(arg, opt->value);
 }
 
 static int option_parse_if_missing(const struct option *opt,
 				   const char *arg, int unset UNUSED)
 {
 	/* unset implies NULL arg, which is handled in our helper */
-	return trailer_set_if_missing(opt->value, arg);
+	return trailer_set_if_missing(arg, opt->value);
 }
 
 static int option_parse_trailer(const struct option *opt,
diff --git a/trailer.c b/trailer.c
index b0b067ab12c..7b0bdfcb27e 100644
--- a/trailer.c
+++ b/trailer.c
@@ -380,7 +380,7 @@  void process_trailers_lists(struct list_head *head,
 	}
 }
 
-int trailer_set_where(enum trailer_where *item, const char *value)
+int trailer_set_where(const char *value, enum trailer_where *item)
 {
 	if (!value)
 		*item = WHERE_DEFAULT;
@@ -397,7 +397,7 @@  int trailer_set_where(enum trailer_where *item, const char *value)
 	return 0;
 }
 
-int trailer_set_if_exists(enum trailer_if_exists *item, const char *value)
+int trailer_set_if_exists(const char *value, enum trailer_if_exists *item)
 {
 	if (!value)
 		*item = EXISTS_DEFAULT;
@@ -416,7 +416,7 @@  int trailer_set_if_exists(enum trailer_if_exists *item, const char *value)
 	return 0;
 }
 
-int trailer_set_if_missing(enum trailer_if_missing *item, const char *value)
+int trailer_set_if_missing(const char *value, enum trailer_if_missing *item)
 {
 	if (!value)
 		*item = MISSING_DEFAULT;
@@ -520,18 +520,18 @@  static int git_trailer_default_config(const char *conf_key, const char *value,
 	variable_name = strrchr(trailer_item, '.');
 	if (!variable_name) {
 		if (!strcmp(trailer_item, "where")) {
-			if (trailer_set_where(&default_trailer_conf.where,
-					      value) < 0)
+			if (trailer_set_where(value,
+					      &default_trailer_conf.where) < 0)
 				warning(_("unknown value '%s' for key '%s'"),
 					value, conf_key);
 		} else if (!strcmp(trailer_item, "ifexists")) {
-			if (trailer_set_if_exists(&default_trailer_conf.if_exists,
-						  value) < 0)
+			if (trailer_set_if_exists(value,
+						  &default_trailer_conf.if_exists) < 0)
 				warning(_("unknown value '%s' for key '%s'"),
 					value, conf_key);
 		} else if (!strcmp(trailer_item, "ifmissing")) {
-			if (trailer_set_if_missing(&default_trailer_conf.if_missing,
-						   value) < 0)
+			if (trailer_set_if_missing(value,
+						   &default_trailer_conf.if_missing) < 0)
 				warning(_("unknown value '%s' for key '%s'"),
 					value, conf_key);
 		} else if (!strcmp(trailer_item, "separators")) {
@@ -600,15 +600,15 @@  static int git_trailer_config(const char *conf_key, const char *value,
 		conf->cmd = xstrdup(value);
 		break;
 	case TRAILER_WHERE:
-		if (trailer_set_where(&conf->where, value))
+		if (trailer_set_where(value, &conf->where))
 			warning(_("unknown value '%s' for key '%s'"), value, conf_key);
 		break;
 	case TRAILER_IF_EXISTS:
-		if (trailer_set_if_exists(&conf->if_exists, value))
+		if (trailer_set_if_exists(value, &conf->if_exists))
 			warning(_("unknown value '%s' for key '%s'"), value, conf_key);
 		break;
 	case TRAILER_IF_MISSING:
-		if (trailer_set_if_missing(&conf->if_missing, value))
+		if (trailer_set_if_missing(value, &conf->if_missing))
 			warning(_("unknown value '%s' for key '%s'"), value, conf_key);
 		break;
 	default:
diff --git a/trailer.h b/trailer.h
index af55032625d..4193bedbae4 100644
--- a/trailer.h
+++ b/trailer.h
@@ -28,9 +28,9 @@  enum trailer_if_missing {
 	MISSING_DO_NOTHING
 };
 
-int trailer_set_where(enum trailer_where *item, const char *value);
-int trailer_set_if_exists(enum trailer_if_exists *item, const char *value);
-int trailer_set_if_missing(enum trailer_if_missing *item, const char *value);
+int trailer_set_where(const char *, enum trailer_where *);
+int trailer_set_if_exists(const char *, enum trailer_if_exists *);
+int trailer_set_if_missing(const char *, enum trailer_if_missing *);
 
 void trailer_set_conf_where(enum trailer_where, struct trailer_conf *);
 void trailer_set_conf_if_exists(enum trailer_if_exists, struct trailer_conf *);