From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Denk <wd@denx.de>
Date: Sun, 14 Jun 2009 01:14:21 +0200
Subject: [U-Boot] [RFC/PATCH 1/2] Add menu Framework
In-Reply-To: <1244920382-21434-1-git-send-email-plagnioj@jcrosoft.com>
References: <20090613191011.GK25406@game.jcrosoft.org>
	<1244920382-21434-1-git-send-email-plagnioj@jcrosoft.com>
Message-ID: <20090613231421.571A1832E416@gemini.denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <1244920382-21434-1-git-send-email-plagnioj@jcrosoft.com> you wrote:
> Introduce a menu framework that allow us to create list menu to simplify
> u-boot and make it more convivial for the end-user.
> 
> This kind of menu is very usefull when you do not have a keyboard or a
> serial console attached to your board to allow you to interract with
> u-boot
> 
> For the develloper part,
> The framework introduce two API
> 
> 1) C
> that allow you to create menu, submenu, entry and complex menu action
> 
> 2) Command
> that allow you as the C API to create menu, submenu, entry and complex
> menu action but this time the actions will be store in a env var and
> then be evaluated and excecuted.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

What's the sense of posting a summary when you repeat the whole text
identically here?

> +		while ((*dst = *src) != '\0') {
> +			dst++;
> +			src++;
> +		}
> +		*dst = (i == (argc - 1)) ? '\0' : ' ';
> +		dst++;

This code repeats. Please factor out.

> +	if (!strncmp(argv[0], "a", 1))
> +		return do_menu_entry_add(cmdtp, flag, argc, argv);
> +	if (!strncmp(argv[0], "l", 1))
> +		return do_menu_entry_list(cmdtp, flag, argc, argv);
> +	else
> +		cmd_usage(cmdtp);
> +	return 0;
...
> +#if defined(CONFIG_CMD_MENU_MANAGEMENT)
> +	if (!strncmp(argv[0], "a", 1))
> +		return do_menu_add(cmdtp, flag, argc, argv);
> +	if (!strncmp(argv[0], "e", 1))
> +		return do_menu_entry(cmdtp, flag, argc, argv);
> +#endif
> +	if (!strncmp(argv[0], "l", 1))
> +		return do_menu_list(cmdtp, flag, argc, argv);
> +	if (!strncmp(argv[0], "s", 1))
> +		return do_menu_show(cmdtp, flag, argc, argv);
> +	else
> +		cmd_usage(cmdtp);

Please use the existing command parser for such subcommands.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You can't have everything... where would you put it?  - Steven Wright