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

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

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

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

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-com... 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.

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-com... 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
participants (3)
-
Heinrich Schuchardt
-
Tom Rini
-
Wolfgang Denk