This is technically a v2 of Message ID: <20240717154858.7505-1-mail@gjnoonan.co.uk> 

However I have changed more than set out in that patch so it is now a series:

  * The logic checkyes using to check the input is a valid variation of
    what we deem yes/truthy has changed.

  * checkyes has been taught that any variation of the word "true" is valid as
    yes.

  * The check yes function has been renamed to check_yes to ensure consistency
    with the rest of the codebase.

I initially changed the function to switch on the length of the string to check
between 1, yes, or true. However since we shouldn't (famous last words) be
adding any more possible values. the new inline isYes is fine. Before we check
we now lowercase the whole string, as opposed to lowercasing each character
individually as before.

Gavin-John Noonan (3):
  cmdconfig: simplify checkyes
  cmdconfig: teach checkyes that true means yes
  cmdconfig: rename checkyes for consistency

 src/cmd/cmdconfig.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

-- 
2.45.2

Rather than having to lowercase each character during the check, we now
lowercase the whole string first, then check if it is correct.

We don't have many variations of what we consider to be yes.

Signed-off-by: Gavin-John Noonan <mail@gjnoonan.co.uk>
---
 src/cmd/cmdconfig.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/cmd/cmdconfig.c b/src/cmd/cmdconfig.c
index d3b7307c..4217d567 100644
--- a/src/cmd/cmdconfig.c
+++ b/src/cmd/cmdconfig.c
@@ -435,20 +435,20 @@ ensure_config(struct gcli_ctx *ctx)
 static int
 checkyes(char const *const tmp)
 {
-	static char const *const yeses[] = { "1", "y", "Y" };
+	size_t tmplen = strlen(tmp) + 1;
+	char *tmp_lower = malloc(tmplen);
 
-	if (strlen(tmp) == 3) {
-		if (tolower(tmp[0]) == 'y' && tolower(tmp[1]) == 'e' &&
-		    tolower(tmp[2]) == 's')
-			return 1;
+	strncpy(tmp_lower, tmp, tmplen);
+
+	for (size_t i = 0; i < tmplen - 1; ++i) {
+		tmp_lower[i] = tolower(tmp_lower[i]);
 	}
 
-	for (size_t i = 0; i < ARRAY_SIZE(yeses); ++i) {
-		if (strcmp(yeses[i], tmp) == 0)
-			return 1;
-	}
+	int isYes = strcmp(tmp_lower, "1") == 0 ||
+		strcmp(tmp_lower, "yes") == 0 ||
 
-	return 0;
+	free(tmp_lower);
+	return isYes;
 }
 
 /* readenv: Read values of environment variables and pre-populate the
-- 
2.45.2

Currently when parsing config and checking for a truthy value we only
allow a variation of the word "yes" (e.g: yEs, YeS, etc) or the numeral
1, but not actually the word true.

Here we teach the checkyes function to also check for a variation of the
word true (e.g: true, True, TrUe, etc)

Signed-off-by: Gavin-John Noonan <mail@gjnoonan.co.uk>
---
 src/cmd/cmdconfig.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/cmd/cmdconfig.c b/src/cmd/cmdconfig.c
index 4217d567..e5628975 100644
--- a/src/cmd/cmdconfig.c
+++ b/src/cmd/cmdconfig.c
@@ -446,6 +446,7 @@ checkyes(char const *const tmp)
 
 	int isYes = strcmp(tmp_lower, "1") == 0 ||
 		strcmp(tmp_lower, "yes") == 0 ||
+		strcmp(tmp_lower, "true") == 0;
 
 	free(tmp_lower);
 	return isYes;
-- 
2.45.2

Throughout the codebase we generally use snake_case for functions.

Signed-off-by: Gavin-John Noonan <mail@gjnoonan.co.uk>
---
 src/cmd/cmdconfig.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/cmd/cmdconfig.c b/src/cmd/cmdconfig.c
index e5628975..df715f26 100644
--- a/src/cmd/cmdconfig.c
+++ b/src/cmd/cmdconfig.c
@@ -433,7 +433,7 @@ ensure_config(struct gcli_ctx *ctx)
 
 /** Check input for a value that indicates yes/true */
 static int
-checkyes(char const *const tmp)
+check_yes(char const *const tmp)
 {
 	size_t tmplen = strlen(tmp) + 1;
 	char *tmp_lower = malloc(tmplen);
@@ -474,13 +474,13 @@ readenv(struct gcli_config *cfg)
 	 * violate the definition to get expected and sane behaviour. */
 	tmp = getenv("NO_COLOR");
 	if (tmp && tmp[0] != '\0')
-		cfg->colours_disabled = checkyes(tmp);
+		cfg->colours_disabled = check_yes(tmp);
 
 	if ((tmp = getenv("GCLI_NOSPINNER")))
-		cfg->no_spinner = checkyes(tmp);
+		cfg->no_spinner = check_yes(tmp);
 
 	if ((tmp = getenv("GCLI_ENABLE_EXPERIMENTAL")))
-		cfg->enable_experimental = checkyes(tmp);
+		cfg->enable_experimental = check_yes(tmp);
 }
 
 int
@@ -1011,7 +1011,7 @@ gcli_config_display_progress_spinner(struct gcli_ctx *ctx)
 	if (sn_sv_null(cfg_entry))
 		return 1;
 
-	if (checkyes(sn_sv_to_cstr(cfg_entry)))
+	if (check_yes(sn_sv_to_cstr(cfg_entry)))
 		return 0;
 
 	return 1;
@@ -1032,5 +1032,5 @@ gcli_config_enable_experimental(struct gcli_ctx *ctx)
 	if (sn_sv_null(cfg_entry))
 		return false;
 
-	return checkyes(sn_sv_to_cstr(cfg_entry));
+	return check_yes(sn_sv_to_cstr(cfg_entry));
 }
-- 
2.45.2

On Thu Jul 18, 2024 at 5:25 PM UTC, Gavin-John Noonan wrote:
> +	int isYes = strcmp(tmp_lower, "1") == 0 ||
> +		strcmp(tmp_lower, "yes") == 0 ||
Apologies, this must've got messed up during the rebase splitting the 
code. If needed I can send v3, but this is an easy fix when/if you 
apply.

-g

Am Thu, Jul 18, 2024 at 05:45:29PM +0000 schrieb Gavin-John Noonan:
> On Thu Jul 18, 2024 at 5:25 PM UTC, Gavin-John Noonan wrote:
> > +	int isYes = strcmp(tmp_lower, "1") == 0 ||
> > +		strcmp(tmp_lower, "yes") == 0 ||
> Apologies, this must've got messed up during the rebase splitting the
> code. If needed I can send v3, but this is an easy fix when/if you
> apply.

No worries, I fixed this and pushed. Thanks!

Nico
--
Sent from hades / FreeBSD 14.1-RELEASE

Please remember: https://useplaintext.email/#etiquette
HTML-formatted mail will likely end up in the Spam folder.
PGP Key: https://herrhotzenplotz.de/pgp.txt
PGP Fingerprint: 1A0E E08D 9D3B CFB9 8C85  A76E F373 F525 45A2 8D14

Nico Sonack <nsonack@herrhotzenplotz.de>