On 23-07-04, Sascha Hauer wrote: > On Tue, Jul 04, 2023 at 10:32:22AM +0200, Marco Felsch wrote: > > Hi Ahmad, > > > > On 23-07-04, Ahmad Fatoum wrote: > > > On 04.07.23 00:58, Marco Felsch wrote: > > > > Add [[ expression ]] support which allow pattern matching like: > > > > > > > > - [[ "foo1" == "foo*" ]] > > > > - [[ "foo" != "bar" ]] > > > > > > > > Signed-off-by: Marco Felsch <m.felsch@xxxxxxxxxxxxxx> > > > > --- > > > > commands/Kconfig | 13 +++++++++++++ > > > > commands/test.c | 39 ++++++++++++++++++++++++++++++++++----- > > > > 2 files changed, 47 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/commands/Kconfig b/commands/Kconfig > > > > index 4d3ff631a8..615e96aa9d 100644 > > > > --- a/commands/Kconfig > > > > +++ b/commands/Kconfig > > > > @@ -1219,6 +1219,19 @@ config CMD_TEST > > > > !, =, !=, -eq, -ne, -ge, -gt, -le, -lt, -o, -a, -z, -n, -d, -e, > > > > -f, -L; see 'man test' on your PC for more information. > > > > > > > > +if CMD_TEST > > > > + > > > > +config CMD_TEST_BASH_COMP > > > > + tristate > > > > + select CONFIG_FNMATCH > > > > > > select FNMATCH, no CONFIG_ > > > > Right. > > > > > Does this really need its own symbol? Can't you just use CONFIG_GLOB > > > and amend the help text? > > > > The implementation was inspired by busybox and they use fnmatch. > > Checking glob.c I don't see any difference, therefore I would keep the > > fnmatch. > > glob() is the wrong function anyway. glob() expands patterns to existing > files. You want to do pattern matching only not looking into the VFS. > fnmatch() is the right function here. > > Besides, glob() uses fnmatch(), so by using glob() you would add even > more code. > > > > > I wasn't sure if every user need the bash(ism) support and sometimes > > bootloader partitions are really small due to legacy reasons. Therefore > > I went this way to let the user the choice to enable/disable the > > support. > > You can't do much in barebox without globalvar support which already > uses fnmatch(), so it's likely enabled anyway. Ah right, I will drop it then. Thanks, Marco > > Sascha > > > > > + return fnmatch(right_op, left_op, 0); > > > > +#endif > > > > > > Scripts that execute differently depending on a barebox config option > > > is not good user experience. I think it would be better if [[ is an > > > error if support is missing. > > > > I was thinking about a warning which is printed once you use [[ and > > didn't enabled it before. Then I read man bash again and [[ supports > > '=' as well which equals the [ function. Therefore I dropped the > > warning. > > > > > Also can't the #ifdef be replaced with a IS_ENABLED() > > > > I don't think so, since the fnmatch is added conditional to keep the > > code base the same if not enabled. > > Use IS_ENABLED(). The linker will discard unused functions for you. We > use this all over the place. > > Sascha > > -- > Pengutronix e.K. | | > Steuerwalder Str. 21 | http://www.pengutronix.de/ | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | >
