From: Linus Arver <linusa@xxxxxxxxxx>
Do not use *_DEFAULT as a suffix to the enums, because the word
"default" is overloaded. The following are two examples of the ambiguity
of the word "default":
(1) "Default" can mean using the "default" values that are hardcoded
in trailer.c as
default_conf_info.where = WHERE_END;
default_conf_info.if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR;
default_conf_info.if_missing = MISSING_ADD;
in ensure_configured(). These values are referred to as "the
default" in the docs for interpret-trailers. These defaults are used
if no "trailer.*" configurations are defined.
(2) "Default" can also mean the "trailer.*" configurations themselves,
because these configurations are used by "default" (ahead of the
hardcoded defaults in (1)) if no command line arguments are
provided. This concept of defaulting back to the configurations was
introduced in 0ea5292e6b (interpret-trailers: add options for
actions, 2017-08-01).
In addition, the corresponding *_DEFAULT values are chosen when the user
provides the "--no-where", "--no-if-exists", or "--no-if-missing" flags
on the command line. These "--no-*" flags are used to clear previously
provided flags of the form "--where", "--if-exists", and "--if-missing".
Using these "--no-*" flags undoes the specifying of these flags (if
any), so using the word "UNSPECIFIED" is more natural here.
So instead of using "*_DEFAULT", use "*_UNSPECIFIED" because this
signals to the reader that the *_UNSPECIFIED value by itself carries no
meaning (it's a zero value and by itself does not "default" to anything,
necessitating the need to have some other way of getting to a useful
value).
Signed-off-by: Linus Arver <linusa@xxxxxxxxxx>
---
trailer.c | 17 ++++++++++-------
trailer.h | 6 +++---
2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/trailer.c b/trailer.c
index f646e484a23..6ad2fbca942 100644
--- a/trailer.c
+++ b/trailer.c
@@ -388,7 +388,7 @@ static void process_trailers_lists(struct list_head *head,
int trailer_set_where(enum trailer_where *item, const char *value)
{
if (!value)
- *item = WHERE_DEFAULT;
+ *item = WHERE_UNSPECIFIED;
else if (!strcasecmp("after", value))
*item = WHERE_AFTER;
else if (!strcasecmp("before", value))
@@ -405,7 +405,7 @@ int trailer_set_where(enum trailer_where *item, const char *value)
int trailer_set_if_exists(enum trailer_if_exists *item, const char *value)
{
if (!value)
- *item = EXISTS_DEFAULT;
+ *item = EXISTS_UNSPECIFIED;
else if (!strcasecmp("addIfDifferent", value))
*item = EXISTS_ADD_IF_DIFFERENT;
else if (!strcasecmp("addIfDifferentNeighbor", value))
@@ -424,7 +424,7 @@ 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)
{
if (!value)
- *item = MISSING_DEFAULT;
+ *item = MISSING_UNSPECIFIED;
else if (!strcasecmp("doNothing", value))
*item = MISSING_DO_NOTHING;
else if (!strcasecmp("add", value))
@@ -586,7 +586,10 @@ static void ensure_configured(void)
if (configured)
return;
- /* Default config must be setup first */
+ /*
+ * Default config must be setup first. These defaults are used if there
+ * are no "trailer.*" or "trailer.<token>.*" options configured.
+ */
default_conf_info.where = WHERE_END;
default_conf_info.if_exists = EXISTS_ADD_IF_DIFFERENT_NEIGHBOR;
default_conf_info.if_missing = MISSING_ADD;
@@ -701,11 +704,11 @@ static void add_arg_item(struct list_head *arg_head, char *tok, char *val,
new_item->value = val;
duplicate_conf(&new_item->conf, conf);
if (new_trailer_item) {
- if (new_trailer_item->where != WHERE_DEFAULT)
+ if (new_trailer_item->where != WHERE_UNSPECIFIED)
new_item->conf.where = new_trailer_item->where;
- if (new_trailer_item->if_exists != EXISTS_DEFAULT)
+ if (new_trailer_item->if_exists != EXISTS_UNSPECIFIED)
new_item->conf.if_exists = new_trailer_item->if_exists;
- if (new_trailer_item->if_missing != MISSING_DEFAULT)
+ if (new_trailer_item->if_missing != MISSING_UNSPECIFIED)
new_item->conf.if_missing = new_trailer_item->if_missing;
}
list_add_tail(&new_item->list, arg_head);
diff --git a/trailer.h b/trailer.h
index ab2cd017567..a689d768c79 100644
--- a/trailer.h
+++ b/trailer.h
@@ -5,14 +5,14 @@
#include "strbuf.h"
enum trailer_where {
- WHERE_DEFAULT,
+ WHERE_UNSPECIFIED,
WHERE_END,
WHERE_AFTER,
WHERE_BEFORE,
WHERE_START
};
enum trailer_if_exists {
- EXISTS_DEFAULT,
+ EXISTS_UNSPECIFIED,
EXISTS_ADD_IF_DIFFERENT_NEIGHBOR,
EXISTS_ADD_IF_DIFFERENT,
EXISTS_ADD,
@@ -20,7 +20,7 @@ enum trailer_if_exists {
EXISTS_DO_NOTHING
};
enum trailer_if_missing {
- MISSING_DEFAULT,
+ MISSING_UNSPECIFIED,
MISSING_ADD,
MISSING_DO_NOTHING
};
--
gitgitgadget
