Better filter and documentation for qmk find #23711
Conversation
| @@ -11,7 +11,7 @@ | |||
| action='append', | |||
| default=[], | |||
| help= # noqa: `format-python` and `pytest` don't agree here. | |||
| f"Filter the list of keyboards based on their info.json data. Accepts the formats key=value, function(key), or function(key,value), eg. 'features.rgblight=true'. Valid functions are {filter_help()}. May be passed multiple times; all filters need to match. Value may include wildcards such as '*' and '?'." # noqa: `format-python` and `pytest` don't agree here. | |||
| f"Filter the list of keyboards based on their info.json data. Accepts the formats key=value, function(key), or function(key,value), eg. 'features.rgblight=true' or 'length(encoder.rotary, >=20)'. Valid functions are {filter_help()}. May be passed multiple times; all filters need to match. Value may include wildcards such as '*' and '?'." # noqa: `format-python` and `pytest` don't agree here. | |||
There was a problem hiding this comment.
I dont like the special handling here. Imo everything should come from filter_help and be self-contained on the classes
Could implement a get_help() function on the base class, as simply returning the function's name by default, and overwrite when needed. Then, for the sake of readability, print them as (instead of the current single-line approach)
* One function
* Another function
Extra docs
Where the indentation is handled by the class adding a long description and the "common" code looks like
"\n * ".join(klass.get_help() for klass in FilterFunction.__subclasses__())
| @@ -61,10 +61,22 @@ def apply(self, target_info: TargetInfo) -> bool: | |||
|
|
|||
| class Length(FilterFunction): | |||
| func_name = "length" | |||
| funcMap = [ | |||
There was a problem hiding this comment.
Just a nitpick, camelCase looks off when everything else is snake_case... Its the "official" format for vars anyway.
|
|
||
| def apply(self, target_info: TargetInfo) -> bool: | ||
| _kb, _km, info = target_info | ||
| return (self.key in info and len(info[self.key]) == int(self.value)) | ||
| val = int(''.join(c for c in self.value if c.isdigit())) |
There was a problem hiding this comment.
As far as i can tell this is intended to convert the (potentially) {<,<=,>,>=}number into an actual int, but i dont like it because of two things:
- You will mistakenly (and silently) convert weird inputs into numbers, eg:
3a2or3.2will be parsed as32 - Is (IMO) much less readable than just doing
for start, func in self.funcMap: # i would also name this something like "operator"
if self.value.startswith(start):
comp = func
val = int(self.value[len(start):] # remove prefix, then convert to int (or fail trying to do so)Also, the extra parenthesis on the for, if and return look a bit off, but just code style yet again
Description
Types of Changes
Issues Fixed or Closed by This PR
Checklist