Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> writes:
> diff --git a/help.c b/help.c
> index bc15066..75b8d4b 100644
> --- a/help.c
> +++ b/help.c
> @@ -5,26 +5,41 @@
> #include "help.h"
> #include "common-cmds.h"
>
> -/* most GUI terminals set COLUMNS (although some don't export it) */
> -static int term_columns(void)
> +/* cache for term_columns() value. Set on first use or when
> + * installing a pager and replacing stdout.
> + */
Just a style.
/*
* We format multi-line
* comment block like
* this.
*/
> +static int term_columns_cache;
> +
> +/* Return cached value (iff set) or $COLUMNS (iff set and positive) or
> + * ioctl(1, TIOCGWINSZ).ws_col (if positive) or 80.
> + *
> + * $COLUMNS even if set, is usually not exported, so
> + * the variable can be used to override autodection.
> + */
> +int term_columns(void)
> {
> - char *col_string = getenv("COLUMNS");
> - int n_cols;
> + if (term_columns_cache)
> + return term_columns_cache;
>
> - if (col_string && (n_cols = atoi(col_string)) > 0)
> - return n_cols;
> + {
> + char *col_string = getenv("COLUMNS");
> + int n_cols;
> +
> + if (col_string && (n_cols = atoi(col_string)) > 0)
> + return (term_columns_cache = n_cols);
> + }
I can see the justification for the existing one inside the #ifdef below,
but why do you need the extra scope above?
We tend to avoid assignment as a part of another expression. The old code
already violates that convention, but please do not make it worse by
introducing three new ones.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html