All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
To: u-boot@lists.denx.de
Subject: [PATCH v2 2/3] allow positional arguments with "run" command
Date: Mon, 19 Oct 2020 10:31:37 +0200	[thread overview]
Message-ID: <b8e3d765-d0a6-12da-5fc9-3e0da0818cb8@prevas.dk> (raw)
In-Reply-To: <259038.1603092665@gemini.denx.de>

On 19/10/2020 09.31, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <2284dd1d-f20c-6246-805e-55454a581960@prevas.dk> you wrote:
>>
>>> Yes that's good, but is the plan now to take these patches rather than
>>> update to the latest hush? I was wondering is Buzybox has any tests
>>> for hush.
>>
>> Well, updating the whole hush code is not, as I've said before,
>> something I can or will take on me ATM, and it's not even clear that
>> that would automatically provide real shell functions.
> 
> Yes, current versions of busybox hush do implement shell functions;
> tested under Fedora 32:

Not what I meant, of course busybox hush does that. What I meant is that
it is not at all obvious how that support would actually benefit U-Boot.
The problem is how one would go about getting the functions defined. Putting

define_func='func() { do_stuff $1 $2; do_more_stuff $3; }'

in the environment and then having to say

run define_func; func foo ...

does not really look like an improvement to me. In contrast, the current
way of defining "one-liner" functions and running with, well, run, is
quite ergonomic - but I do miss the ability to provide parameters other
than via global settings. With these patches, the above would just be

func='do_stuff $1 $2; do_more_stuff $3;'

run func -- foo ...

I guess one could have something like CONFIG_DOT_PROFILE and have that
point at a script that gets built into the U-Boot binary and sourced at
shell startup; then one could put one's functions in there, or have the
flexibility of having that file load some stdlib.sh from somewhere.

> 
> My big concern here is that adding bells and whistles to our ancient
> version of hush will just make it all the harder to upgrade to a
> recent version.  Already now we have the situation that we must be
> afraid to break existing code which works around some of the
> problems.  Adding more "special code" will not make this better.
> 
> And even if we can upgrade to a new version independently, we still
> might have to keep this workaround code for compatibility, as people
> started using it, and it is not compatible with any existing shell.
> 
> So the _much_ better approach would indeed be to upgrade to a recent
> version of hush.

I agree in principle - there are other shell features I'm also missing
(though see above, I don't immediately see how an upgrade would make
functions available in a useful way).

Someone speaking up and saying "I'm going to look at an overhaul of hush
within the next year or so" would clearly be a strong argument against
inclusion of these patches. Lacking such a pony promise, this really
boils down to an idealist/pragmatist issue, and we can keep going around
this in circles forever, so I think someone (Tom?) should make a decision.

Rasmus

  reply	other threads:[~2020-10-19  8:31 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25 11:19 [PATCH 0/3] add "call" command Rasmus Villemoes
2020-09-25 11:19 ` [PATCH 1/3] cli_hush.c: refactor handle_dollar() to prepare for cmd_call Rasmus Villemoes
2020-09-25 13:02   ` Wolfgang Denk
2020-09-25 11:19 ` [PATCH 2/3] cli_hush.c: add "call" command Rasmus Villemoes
2020-09-25 13:18   ` Rasmus Villemoes
2020-09-26  8:37     ` Wolfgang Denk
2020-09-25 11:19 ` [PATCH 3/3] ut: add small hush tests Rasmus Villemoes
2020-09-25 11:52 ` [PATCH 0/3] add "call" command Heinrich Schuchardt
2020-09-25 12:36   ` Rasmus Villemoes
2020-09-25 13:09   ` Wolfgang Denk
2020-09-25 13:38     ` Rasmus Villemoes
2020-09-25 13:38     ` Heinrich Schuchardt
2020-09-25 13:51       ` Rasmus Villemoes
2020-09-26  8:55         ` Wolfgang Denk
2020-09-26  8:51       ` Wolfgang Denk
2020-09-26 10:39         ` Heinrich Schuchardt
2020-09-26 14:13           ` Wolfgang Denk
2020-09-25 12:59 ` Wolfgang Denk
2020-09-25 14:40   ` Simon Glass
2020-09-26 14:02     ` Wolfgang Denk
2020-09-29 17:45       ` Tom Rini
2020-09-30 11:46         ` Wolfgang Denk
2020-10-07  7:20 ` [PATCH v2 0/3] allow positional arguments with "run" Rasmus Villemoes
2020-10-07  7:20   ` [PATCH v2 1/3] cli_hush.c: refactor handle_dollar() to prepare for "run with arguments" Rasmus Villemoes
2020-10-12  3:34     ` Simon Glass
2020-10-07  7:20   ` [PATCH v2 2/3] allow positional arguments with "run" command Rasmus Villemoes
2020-10-12  3:34     ` Simon Glass
2020-10-12  7:06       ` Rasmus Villemoes
2020-10-15 15:05         ` Simon Glass
2020-10-15 22:06           ` Rasmus Villemoes
2020-10-19  7:31             ` Wolfgang Denk
2020-10-19  8:31               ` Rasmus Villemoes [this message]
2020-10-19  8:49                 ` Wolfgang Denk
2020-10-07  7:20   ` [PATCH v2 3/3] ut: add small hush tests Rasmus Villemoes
2020-10-12  3:34     ` Simon Glass
2020-11-05  7:25   ` [PATCH v2 0/3] allow positional arguments with "run" Rasmus Villemoes
2020-11-06 20:52     ` Tom Rini
2020-11-08 13:28       ` Wolfgang Denk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b8e3d765-d0a6-12da-5fc9-3e0da0818cb8@prevas.dk \
    --to=rasmus.villemoes@prevas.dk \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.