Re: [PATCH 5/6] commands: drvinfo: support filtering by driver

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi!

On 27.10.22 09:29, Sascha Hauer wrote:
> On Wed, Oct 26, 2022 at 08:42:04AM +0200, Ahmad Fatoum wrote:
>> drvinfo can be very long especially for the in-tree defconfigs,
>> add optional filtering support:
>> +	if (IS_ENABLED(CONFIG_AUTO_COMPLETE) && argc > 1)
>> +		filter = strjoin(" ", &argv[1], argc - 1);
> 
> Why does this depend on CONFIG_AUTO_COMPLETE?

drvinfo is something I think should always be enabled and I didn't want
to bloat it unconditionally.

> 
>> +
>>  	printf("Driver\tDevice(s)\n");
>>  	printf("--------------------\n");
>>  	for_each_driver(drv) {
>> +		if (filter && !str_has_prefix(drv->name, filter))
>> +			continue;
> 
> I don't see how this is expected to work. When you pass multiple drivers
> as argument 'filter' will be a concatenation of multiple driver names
> which then matches nothing.

Autocomplete doesn't work for elements with spaces, so when you write

  drvinfo TI<Tab>

You get

    drvinfo TI DP83

with no further ability to complete. The prefix matching can work with that
and will produce:

Driver  Device(s)
--------------------
TI DP83867
TI DP83TD510E

Use 'devinfo DEVICE' for more information

> How about using fnmatch? We could then pass things like "*usb*"

I am content with the tab completion.

Cheers,
Ahmad

> 
> Sascha
> 
> 
>> +
>>  		printf("%s\n",drv->name);
>>  		for_each_device(dev) {
>>  			if (dev->driver == drv)
>> @@ -24,6 +32,7 @@ static int do_drvinfo(int argc, char *argv[])
>>  	if (IS_ENABLED(CONFIG_CMD_DEVINFO))
>>  		printf("\nUse 'devinfo DEVICE' for more information\n");
>>  
>> +	free(filter);
>>  	return 0;
>>  }
>>  
>> @@ -31,5 +40,9 @@ static int do_drvinfo(int argc, char *argv[])
>>  BAREBOX_CMD_START(drvinfo)
>>  	.cmd		= do_drvinfo,
>>  	BAREBOX_CMD_DESC("list compiled-in device drivers")
>> +#ifdef CONFIG_AUTO_COMPLETE
>> +	BAREBOX_CMD_OPTS("[DRIVER]")
>> +#endif
>>  	BAREBOX_CMD_GROUP(CMD_GRP_INFO)
>> +	BAREBOX_CMD_COMPLETE(driver_complete)
>>  BAREBOX_CMD_END
>> diff --git a/common/complete.c b/common/complete.c
>> index ab3c98549314..916d13d776ce 100644
>> --- a/common/complete.c
>> +++ b/common/complete.c
>> @@ -174,6 +174,27 @@ static int device_param_complete(struct device_d *dev, struct string_list *sl,
>>  	return 0;
>>  }
>>  
>> +int driver_complete(struct string_list *sl, char *instr)
>> +{
>> +	struct driver_d *drv;
>> +	int len;
>> +
>> +	if (!instr)
>> +		instr = "";
>> +
>> +	len = strlen(instr);
>> +
>> +	for_each_driver(drv) {
>> +		if (strncmp(instr, drv->name, len))
>> +			continue;
>> +
>> +		string_list_add_asprintf(sl, "%s ", drv->name);
>> +	}
>> +
>> +	return COMPLETE_CONTINUE;
>> +}
>> +EXPORT_SYMBOL(driver_complete);
>> +
>>  int empty_complete(struct string_list *sl, char *instr)
>>  {
>>  	return COMPLETE_END;
>> diff --git a/include/complete.h b/include/complete.h
>> index b0e675b5599f..2068760ac235 100644
>> --- a/include/complete.h
>> +++ b/include/complete.h
>> @@ -14,6 +14,7 @@ void complete_reset(void);
>>  
>>  int command_complete(struct string_list *sl, char *instr);
>>  int device_complete(struct string_list *sl, char *instr);
>> +int driver_complete(struct string_list *sl, char *instr);
>>  int empty_complete(struct string_list *sl, char *instr);
>>  int eth_complete(struct string_list *sl, char *instr);
>>  int command_var_complete(struct string_list *sl, char *instr);
>> -- 
>> 2.30.2
>>
>>
>>
> 


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