[U-Boot] [PATCH 0/2] cmd: auto-complete args starting with a $
Hello,
It's pretty common to pass arguments that start with a $ and are then expanded by the shell, and I'm this kind of lazy guy that hits tab all the time and expects the shell to suggest something appropriate. So here is a patchset adding support for ${} auto-completion and using the new helper from the mtd command.
Using it elsewhere should be pretty easy (I already patched env set to use it too [1]).
Note that this patch series depends on [2].
Regards,
Boris
[1]https://github.com/bbrezillon/u-boot/commit/9f37c5b6471c7479aa043524d31531f0... [2]https://patchwork.ozlabs.org/project/uboot/list/?series=79552
Boris Brezillon (2): common: command: Provide a dollar_complete() helper cmd: mtd: auto-complete args starting with a $ when appropriate
cmd/mtd.c | 40 ++++++++++++++++++++++++++++++++----- common/command.c | 22 ++++++++++++++++++--- env/common.c | 50 ++++++++++++++++++++++++++++++++++++++++++++--- include/command.h | 2 ++ include/common.h | 3 ++- 5 files changed, 105 insertions(+), 12 deletions(-)
Add an helper to auto-complete arguments starting with a '$' with what's available in the environment.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com --- common/command.c | 22 ++++++++++++++++++--- env/common.c | 50 ++++++++++++++++++++++++++++++++++++++++++++--- include/command.h | 2 ++ include/common.h | 3 ++- 4 files changed, 70 insertions(+), 7 deletions(-)
diff --git a/common/command.c b/common/command.c index 19f0534a76ea..754ab9bbc396 100644 --- a/common/command.c +++ b/common/command.c @@ -143,22 +143,38 @@ int cmd_usage(const cmd_tbl_t *cmdtp)
#ifdef CONFIG_AUTO_COMPLETE
+static char env_complete_buf[512]; + int var_complete(int argc, char * const argv[], char last_char, int maxv, char *cmdv[]) { - static char tmp_buf[512]; int space;
space = last_char == '\0' || isblank(last_char);
if (space && argc == 1) - return env_complete("", maxv, cmdv, sizeof(tmp_buf), tmp_buf); + return env_complete("", maxv, cmdv, sizeof(env_complete_buf), + env_complete_buf, false);
if (!space && argc == 2) - return env_complete(argv[1], maxv, cmdv, sizeof(tmp_buf), tmp_buf); + return env_complete(argv[1], maxv, cmdv, + sizeof(env_complete_buf), + env_complete_buf, false);
return 0; }
+int dollar_complete(int argc, char * const argv[], char last_char, int maxv, + char *cmdv[]) +{ + /* Make sure the last argument starts with a $. */ + if (argc < 1 || argv[argc - 1][0] != '$' || + last_char == '\0' || isblank(last_char)) + return 0; + + return env_complete(argv[argc - 1], maxv, cmdv, sizeof(env_complete_buf), + env_complete_buf, true); +} + /*************************************************************************************/
int complete_subcmdv(cmd_tbl_t *cmdtp, int count, int argc, diff --git a/env/common.c b/env/common.c index 3317cef35522..aa9a097bced0 100644 --- a/env/common.c +++ b/env/common.c @@ -241,31 +241,75 @@ void env_relocate(void) }
#if defined(CONFIG_AUTO_COMPLETE) && !defined(CONFIG_SPL_BUILD) -int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf) +int env_complete(char *var, int maxv, char *cmdv[], int bufsz, char *buf, + bool dollar_comp) { ENTRY *match; int found, idx;
+ if (dollar_comp) { + /* + * When doing $ completion, the first character should + * obviously be a '$'. + */ + if (var[0] != '$') + return 0; + + var++; + + /* + * The second one, if present, should be a '{', as some + * configuration of the u-boot shell expand ${var} but not + * $var. + */ + if (var[0] == '{') + var++; + else if (var[0] != '\0') + return 0; + } + idx = 0; found = 0; cmdv[0] = NULL;
+ while ((idx = hmatch_r(var, idx, &match, &env_htab))) { int vallen = strlen(match->key) + 1;
- if (found >= maxv - 2 || bufsz < vallen) + if (found >= maxv - 2 || + bufsz < vallen + (dollar_comp ? 3 : 0)) break;
cmdv[found++] = buf; + + /* Add the '${' prefix to each var when doing $ completion. */ + if (dollar_comp) { + strcpy(buf, "${"); + buf += 2; + bufsz -= 3; + } + memcpy(buf, match->key, vallen); buf += vallen; bufsz -= vallen; + + if (dollar_comp) { + /* + * This one is a bit odd: vallen already contains the + * '\0' character but we need to add the '}' suffix, + * hence the buf - 1 here. strcpy() will add the '\0' + * character just after '}'. buf is then incremented + * to account for the extra '}' we just added. + */ + strcpy(buf - 1, "}"); + buf++; + } }
qsort(cmdv, found, sizeof(cmdv[0]), strcmp_compar);
if (idx) - cmdv[found++] = "..."; + cmdv[found++] = dollar_comp ? "${...}" : "...";
cmdv[found] = NULL; return found; diff --git a/include/command.h b/include/command.h index 461b17447c0d..c512ec6854d0 100644 --- a/include/command.h +++ b/include/command.h @@ -84,6 +84,8 @@ static inline bool cmd_is_repeatable(cmd_tbl_t *cmdtp)
#ifdef CONFIG_AUTO_COMPLETE extern int var_complete(int argc, char * const argv[], char last_char, int maxv, char *cmdv[]); +int dollar_complete(int argc, char * const argv[], char last_char, int maxv, + char *cmdv[]); extern int cmd_auto_complete(const char *const prompt, char *buf, int *np, int *colp); #endif
diff --git a/include/common.h b/include/common.h index 8b561370326f..2c776adc5fe0 100644 --- a/include/common.h +++ b/include/common.h @@ -237,7 +237,8 @@ static inline int env_set_addr(const char *varname, const void *addr) }
#ifdef CONFIG_AUTO_COMPLETE -int env_complete(char *var, int maxv, char *cmdv[], int maxsz, char *buf); +int env_complete(char *var, int maxv, char *cmdv[], int maxsz, char *buf, + bool dollar_comp); #endif int get_env_id (void);
It's quite usual to have RAM or flash address stored in env vars. Use $ auto-completion for such arguments.
Signed-off-by: Boris Brezillon boris.brezillon@bootlin.com --- cmd/mtd.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/cmd/mtd.c b/cmd/mtd.c index cda702d18b16..b13dbef3e888 100644 --- a/cmd/mtd.c +++ b/cmd/mtd.c @@ -517,6 +517,36 @@ static int mtd_name_complete(int argc, char * const argv[], char last_char,
return n_found; } + +static int subcmd_complete(int maxargs, int argc, char * const argv[], + char last_char, int maxv, char *cmdv[]) +{ + if (argc > maxargs) + return 0; + + if (argc <= 2) + return mtd_name_complete(argc, argv, last_char, maxv, cmdv); + + return dollar_complete(argc, argv, last_char, maxv, cmdv); +} + +static int four_args_complete(int argc, char * const argv[], char last_char, + int maxv, char *cmdv[]) +{ + return subcmd_complete(5, argc, argv, last_char, maxv, cmdv); +} + +static int three_args_complete(int argc, char * const argv[], char last_char, + int maxv, char *cmdv[]) +{ + return subcmd_complete(4, argc, argv, last_char, maxv, cmdv); +} + +static int one_arg_complete(int argc, char * const argv[], char last_char, + int maxv, char *cmdv[]) +{ + return subcmd_complete(2, argc, argv, last_char, maxv, cmdv); +} #endif /* CONFIG_AUTO_COMPLETE */
static char mtd_help_text[] = @@ -548,12 +578,12 @@ static char mtd_help_text[] = U_BOOT_CMD_WITH_SUBCMDS(mtd, "MTD utils", mtd_help_text, U_BOOT_SUBCMD_MKENT(list, 1, 1, do_mtd_list), U_BOOT_SUBCMD_MKENT_COMPLETE(read, 5, 0, do_mtd_io, - mtd_name_complete), + four_args_complete), U_BOOT_SUBCMD_MKENT_COMPLETE(write, 5, 0, do_mtd_io, - mtd_name_complete), + four_args_complete), U_BOOT_SUBCMD_MKENT_COMPLETE(dump, 4, 0, do_mtd_io, - mtd_name_complete), + three_args_complete), U_BOOT_SUBCMD_MKENT_COMPLETE(erase, 4, 0, do_mtd_erase, - mtd_name_complete), + three_args_complete), U_BOOT_SUBCMD_MKENT_COMPLETE(bad, 2, 1, do_mtd_bad, - mtd_name_complete)); + one_arg_complete));
Dear Boris,
In message 20181203220726.19370-1-boris.brezillon@bootlin.com you wrote:
It's pretty common to pass arguments that start with a $ and are then expanded by the shell, and I'm this kind of lazy guy that hits tab all the time and expects the shell to suggest something appropriate. So here is a patchset adding support for ${} auto-completion and using the new helper from the mtd command.
You mean, this feature depends on MTD support? this is not goot.
Also, you should make this feature configurable. Not everybody may want to use it or may have the memory available.
Technically - should autocompletion not be prevented for escaped '$' characters, i. e. hitting TAB after a '$' sequence should NOT expand?
Best regards,
Wolfgang Denk
On Tue, 04 Dec 2018 10:44:19 +0100 Wolfgang Denk wd@denx.de wrote:
Dear Boris,
In message 20181203220726.19370-1-boris.brezillon@bootlin.com you wrote:
It's pretty common to pass arguments that start with a $ and are then expanded by the shell, and I'm this kind of lazy guy that hits tab all the time and expects the shell to suggest something appropriate. So here is a patchset adding support for ${} auto-completion and using the new helper from the mtd command.
You mean, this feature depends on MTD support? this is not goot.
Not at all, MTD is just the first user of this dollar_complete() helper. That's what I meant.
Also, you should make this feature configurable. Not everybody may want to use it or may have the memory available.
It already depends on CONFIG_AUTO_COMPLETE, and each command has to explicitly call the helper to support ${} auto-completion. Not that $ auto-completion only makes sense for some arguments, not all of them. It's the responsibility of the command itself to decide when it should be used.
Technically - should autocompletion not be prevented for escaped '$' characters, i. e. hitting TAB after a '$' sequence should NOT expand?
It's already the case. The argument has to start with $ or ${ to be auto-completed.
Dear Boris,
In message 20181204105448.63b9af8c@bbrezillon you wrote:
It's pretty common to pass arguments that start with a $ and are then expanded by the shell, and I'm this kind of lazy guy that hits tab all the time and expects the shell to suggest something appropriate. So here is a patchset adding support for ${} auto-completion and using the new helper from the mtd command.
You mean, this feature depends on MTD support? this is not goot.
Not at all, MTD is just the first user of this dollar_complete() helper. That's what I meant.
But is this not based on the code of mtd_name_complete() which is only availabole when MTD is present?
Also, you should make this feature configurable. Not everybody may want to use it or may have the memory available.
It already depends on CONFIG_AUTO_COMPLETE, and each command has to explicitly call the helper to support ${} auto-completion. Not that $ auto-completion only makes sense for some arguments, not all of them. It's the responsibility of the command itself to decide when it should be used.
Um... this is not hat I would expect - if we implement auto-completion for variables, it should IMO work similar to what a standard shell is doing.
In Bash, I can do:
-> foo=1234 -> echo $f<TAB> $f $flag $foo -> echo $f
i. e. this is a feature of the shell and not of any command. Implementing this a zillion times for each of the commands in inacceptable. Also, implementing it for one command and not for another makese no sense - that would violate the Principle of Least Surprise, as you have something which looks convenient but does not work everywhere.
Technically - should autocompletion not be prevented for escaped '$' characters, i. e. hitting TAB after a '$' sequence should NOT expand?
It's already the case. The argument has to start with $ or ${ to be auto-completed.
We should implement this such that behaviour is similar to what we see on a standard shell.
Best regards,
Wolfgang Denk
Hi Wolfgang,
On Tue, 04 Dec 2018 11:14:31 +0100 Wolfgang Denk wd@denx.de wrote:
Dear Boris,
In message 20181204105448.63b9af8c@bbrezillon you wrote:
It's pretty common to pass arguments that start with a $ and are then expanded by the shell, and I'm this kind of lazy guy that hits tab all the time and expects the shell to suggest something appropriate. So here is a patchset adding support for ${} auto-completion and using the new helper from the mtd command.
You mean, this feature depends on MTD support? this is not goot.
Not at all, MTD is just the first user of this dollar_complete() helper. That's what I meant.
But is this not based on the code of mtd_name_complete() which is only availabole when MTD is present?
Nope. See patch 1, the code is completely independent from the mtd cmd.
Also, you should make this feature configurable. Not everybody may want to use it or may have the memory available.
It already depends on CONFIG_AUTO_COMPLETE, and each command has to explicitly call the helper to support ${} auto-completion. Not that $ auto-completion only makes sense for some arguments, not all of them. It's the responsibility of the command itself to decide when it should be used.
Um... this is not hat I would expect - if we implement auto-completion for variables, it should IMO work similar to what a standard shell is doing.
In Bash, I can do:
-> foo=1234 -> echo $f<TAB> $f $flag $foo
And that's pretty much how it works except that right now, echo has to be patched to support $ auto-completion.
-> echo $f
For the record, not all shell configs expand $foo, some require that foo be enclosed in ${} (${foo}), that's why the current proposal always auto-completes $ as ${.
i. e. this is a feature of the shell and not of any command. Implementing this a zillion times for each of the commands in inacceptable. Also, implementing it for one command and not for another makese no sense - that would violate the Principle of Least Surprise, as you have something which looks convenient but does not work everywhere.
Okay, I understand. I can try to hook that up at the common/command.c level.
Technically - should autocompletion not be prevented for escaped '$' characters, i. e. hitting TAB after a '$' sequence should NOT expand?
It's already the case. The argument has to start with $ or ${ to be auto-completed.
We should implement this such that behaviour is similar to what we see on a standard shell.
Apart from the "make that available to everyone" comment, is there anything else you think should be changed? IOW, what is not compliant with a standard shell auto-completion in my proposal?
Regards,
Boris
Dear Boris,
In message 20181204113313.577178ac@bbrezillon you wrote:
But is this not based on the code of mtd_name_complete() which is only availabole when MTD is present?
Nope. See patch 1, the code is completely independent from the mtd cmd.
OK, then I misread the patches.
Apart from the "make that available to everyone" comment, is there anything else you think should be changed? IOW, what is not compliant with a standard shell auto-completion in my proposal?
I can't say easily from the code. I'd have to see this running (in the sandbox, for example).
Best regards,
Wolfgang Denk
On Tue, 04 Dec 2018 14:00:47 +0100 Wolfgang Denk wd@denx.de wrote:
Dear Boris,
In message 20181204113313.577178ac@bbrezillon you wrote:
But is this not based on the code of mtd_name_complete() which is only availabole when MTD is present?
Nope. See patch 1, the code is completely independent from the mtd cmd.
OK, then I misread the patches.
Apart from the "make that available to everyone" comment, is there anything else you think should be changed? IOW, what is not compliant with a standard shell auto-completion in my proposal?
I can't say easily from the code. I'd have to see this running (in the sandbox, for example).
Actually, that's how I tested it. You can easily test it with the echo command after applying patch 1 of this series + the following diff.
--->8--- diff --git a/cmd/echo.c b/cmd/echo.c index 5b018d9349ab..857f268c6a61 100644 --- a/cmd/echo.c +++ b/cmd/echo.c @@ -47,9 +47,10 @@ static int do_echo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return 0; }
-U_BOOT_CMD( +U_BOOT_CMD_COMPLETE( echo, CONFIG_SYS_MAXARGS, 1, do_echo, "echo args to console", "[args..]\n" - " - echo args to console; \c suppresses newline" + " - echo args to console; \c suppresses newline", + dollar_complete );
On Tue, 4 Dec 2018 11:33:13 +0100 Boris Brezillon boris.brezillon@bootlin.com wrote:
i. e. this is a feature of the shell and not of any command. Implementing this a zillion times for each of the commands in inacceptable. Also, implementing it for one command and not for another makese no sense - that would violate the Principle of Least Surprise, as you have something which looks convenient but does not work everywhere.
Okay, I understand. I can try to hook that up at the common/command.c level.
I thought it would be much more complicated: here is the diff adding $ auto-completion for everyone. The only drawback I see with this approach is that cmd/sub-cmd arg auto-completion cannot be bound to cmd->maxargs anymore, but I guess that's acceptable.
--->8--- diff --git a/common/command.c b/common/command.c index 754ab9bbc396..b9a44d4ea7a8 100644 --- a/common/command.c +++ b/common/command.c @@ -373,9 +373,14 @@ int cmd_auto_complete(const char *const prompt, char *buf, int *np, int *colp) /* separate into argv */ argc = make_argv(tmp_buf, sizeof(argv)/sizeof(argv[0]), argv);
- /* do the completion and return the possible completions */ - i = complete_cmdv(argc, argv, last_char, - sizeof(cmdv) / sizeof(cmdv[0]), cmdv); + /* first try a $ completion */ + i = dollar_complete(argc, argv, last_char, + sizeof(cmdv) / sizeof(cmdv[0]), cmdv); + if (!i) { + /* do the completion and return the possible completions */ + i = complete_cmdv(argc, argv, last_char, + sizeof(cmdv) / sizeof(cmdv[0]), cmdv); + }
/* no match; bell and out */ if (i == 0) {
participants (2)
-
Boris Brezillon -
Wolfgang Denk