[U-Boot] [PATCH 1/1] hush: provide help for 'if', 'for', and 'while'

[U-Boot] [PATCH 1/1] hush: provide help for 'if', 'for', and 'while'

[Heinrich Schuchardt]

Provide online help for hush commands 'if', 'for', and 'while'.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 common/cli_hush.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/common/cli_hush.c b/common/cli_hush.c
index 955e8fe536..d7dfa8a75a 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -3711,5 +3711,24 @@ U_BOOT_CMD(
 	"    - print value of hushshell variable 'name'"
 );

+U_BOOT_CMD(
+	for, 0, 0, NULL,
+	"execute command for each member of a list",
+	"NAME in WORDS; do COMMANDS; done"
+);
+
+U_BOOT_CMD(
+	if, 0, 0, NULL,
+	"execute commands based on condition",
+	"COMMANDS; then COMMANDS; "
+	"[ elif COMMANDS; then COMMANDS; ]... [ else COMMANDS; ] fi"
+);
+
+U_BOOT_CMD(
+	while, 0, 0, NULL,
+	"execute commands as long as a test succeeds",
+	"COMMANDS; do COMMANDS; done"
+);
+
 #endif
 /****************************************************************************/
--
2.20.1

[U-Boot] [PATCH 1/1] hush: provide help for 'if', 'for', and 'while'

[Wolfgang Denk]

Dear Heinrich,

In message <20190329113408.2168-1-xypron.glpk@gmx.de> you wrote:
> Provide online help for hush commands 'if', 'for', and 'while'.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Why for these, and not for the rest of the shell syntax?

This does not make sense to me.  Shell syntax is way more complex
than you can explain here.  In the end, you are just adding to the
memory footprint for minimal (or no) advantage.

I'm against adding this.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
It may be that your whole purpose in life is simply  to  serve  as  a
warning to others.

[U-Boot] [PATCH 1/1] hush: provide help for 'if', 'for', and 'while'

[Heinrich Schuchardt]

On 3/29/19 1:11 PM, Wolfgang Denk wrote:
> Dear Heinrich,
>
> In message <20190329113408.2168-1-xypron.glpk@gmx.de> you wrote:
>> Provide online help for hush commands 'if', 'for', and 'while'.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> Why for these, and not for the rest of the shell syntax?

I missed one keyword "until" which I should add. But otherwise this
patch covers all keywords defined in cli_hush.c.

It does not cover logical expression (&&, ||) and comparisons.

>
> This does not make sense to me.  Shell syntax is way more complex
> than you can explain here.  In the end, you are just adding to the
> memory footprint for minimal (or no) advantage.

You could say the same for any online help. I do not understand why you
consider these commands not worth a description while we provide online
help for all other commands including "test", "echo", "help" and even "?".

Best regards

Heinrich

>
> I'm against adding this.
>
> Best regards,
>
> Wolfgang Denk
>

[U-Boot] [PATCH 1/1] hush: provide help for 'if', 'for', and 'while'

[Tom Rini]

On Fri, Mar 29, 2019 at 08:21:01PM +0100, Heinrich Schuchardt wrote:
> On 3/29/19 1:11 PM, Wolfgang Denk wrote:
> > Dear Heinrich,
> >
> > In message <20190329113408.2168-1-xypron.glpk@gmx.de> you wrote:
> >> Provide online help for hush commands 'if', 'for', and 'while'.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >
> > Why for these, and not for the rest of the shell syntax?
> 
> I missed one keyword "until" which I should add. But otherwise this
> patch covers all keywords defined in cli_hush.c.
> 
> It does not cover logical expression (&&, ||) and comparisons.
> 
> >
> > This does not make sense to me.  Shell syntax is way more complex
> > than you can explain here.  In the end, you are just adding to the
> > memory footprint for minimal (or no) advantage.
> 
> You could say the same for any online help. I do not understand why you
> consider these commands not worth a description while we provide online
> help for all other commands including "test", "echo", "help" and even "?".
> 
> Best regards
> 
> Heinrich
> 
> >
> > I'm against adding this.
> >
> > Best regards,
> >
> > Wolfgang Denk

Maybe a tiny bit more context is useful here.  Over at
https://stackoverflow.com/questions/55381641/how-to-test-the-return-of-a-command-in-u-boot-cli
Heinrich's initial answer was different as he didn't know we had 'if'
under HUSH_PARSER.  The initial poster also didn't have any idea on how
the syntax for 'if' worked for us.  That tells me there's probably other
folks out there that also don't know and we should provide a little
help.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190331/084ebcf6/attachment.sig>

[U-Boot] [PATCH 1/1] hush: provide help for 'if', 'for', and 'while'

[Wolfgang Denk]

Dear Tom,

In message <20190331113756.GG18421@bill-the-cat> you wrote:
> 
> > You could say the same for any online help. I do not understand why you
> > consider these commands not worth a description while we provide online
> > help for all other commands including "test", "echo", "help" and even "?".

All the other commands are U-Boot specific.  The Hush shell is not -
it has been borrowed from elsewhere (Busybox) and the features
Heinrich finds worth documenting are these of more or less any other
(non-C) shell.

> Maybe a tiny bit more context is useful here.  Over at
> https://stackoverflow.com/questions/55381641/how-to-test-the-return-of-a-command-in-u-boot-cli
> Heinrich's initial answer was different as he didn't know we had 'if'
> under HUSH_PARSER.  The initial poster also didn't have any idea on how
> the syntax for 'if' worked for us.  That tells me there's probably other
> folks out there that also don't know and we should provide a little
> help.

Is argument backfires on two accounts:  First, the original poster
already _knew_ that we have "if" / "then" / "else"; what he didn't
know is shell syntax in general; I have no idea how he came up with
something like "test {gpio status 50}" - this doesn;t work in any
command interpeter I know.  Second, what this original poster was
trying to do is command substitution (I think), which we don;t
have.

So the patch explains hat this poster already knew, and fails to
explain that we don't have any redirection or command substitution.

Existing U-Boot documentation mantions in several places that there
are two command line interfaces: a very simple one, and the Hush
shell.  So if you need information about Hush shell syntax, the
usual approach would be something like [1], which shows [2] as first
link, and [3] as second link - and this explanation is definitely
more helpful than the suggested patch.

Even with the patch there is - for example - still no explanation in
the online help about the difference between environment variables
and shell variables.

Yes, I agree, we need (more and better) documentation.  But not
everything needs to be documented as part of the online help.

This patch just costs memory footprint for extremely little (and
questionable) benefit.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
Brain fried - Core dumped