[PATCH v2 2/2] cmd: ufetch: only show comma separator if there was a previous feature

[Simon Glass]

Hi Quentin,

On 2026-05-04T14:48:27, Quentin Schulz <foss+uboot at 0leil.net> wrote:
> cmd: ufetch: only show comma separator if there was a previous feature
>
> Currently, if NET is disabled, the next feature to be printed will start
> with a comma and a space which is not pretty. Add the comma and
> whitespace only when a previous feature has already been shown.
>
> Signed-off-by: Quentin Schulz <quentin.schulz at cherry.de>
>
> cmd/ufetch.c | 29 ++++++++++++++++++++---------
>  1 file changed, 20 insertions(+), 9 deletions(-)

> diff --git a/cmd/ufetch.c b/cmd/ufetch.c
> @@ -157,26 +157,37 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
> -             case FEATURES:
> +             case FEATURES: {
> +                     bool sep = false;
> +
>                       printf('Features:' RESET " ");
> -                     if (IS_ENABLED(CONFIG_NET))
> -                             printf('Net');
> -                     if (IS_ENABLED(CONFIG_EFI_LOADER))
> -                             printf(", EFI");
> -                     if (IS_ENABLED(CONFIG_CMD_CAT))
> -                             printf(", cat :3");
> +                     if (IS_ENABLED(CONFIG_NET)) {
> +                             printf('%sNet', sep ? ", " : "");
> +                             sep = true;
> +                     }
> +                     if (IS_ENABLED(CONFIG_EFI_LOADER)) {
> +                             printf('%sEFI', sep ? ", " : "");
> +                             sep = true;
> +                     }
> +                     if (IS_ENABLED(CONFIG_CMD_CAT)) {
> +                             printf("%scat :3", sep ? ", " : "");
> +                             sep = true;
> +                     }

Sorry, one more thought: you couldkeep the separator as a string that
starts empty and becomes ", " after the first print:

    const char *sep = "";

    if (IS_ENABLED(CONFIG_NET)) {
        printf('%sNet', sep);
        sep = ", ";
    }

> diff --git a/cmd/ufetch.c b/cmd/ufetch.c
> @@ -157,26 +157,37 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
>  #ifdef CONFIG_ARM64
>                       switch (current_el()) {
>                       case 2:
> -                             printf(", VMs");
> +                             printf('%sVMs', sep ? ", " : "");
> +                             sep = true;
>                               break;
>                       case 3:
> -                             printf(", full control!");
> +                             printf("%sfull control!", sep ? ", " : "");
> +                             sep = true;
>                               break;
>                       }
>  #endif

nit: the sep = true is dead since nothing follows the switch.

Reviewed-by: Simon Glass <sjg at chromium.org>

Regards,
Simon