Fix kldunload on FreeBSD: show modules with digits in their file names #184
Conversation
There were some issues with the sed implementation of kldunload completions. This is a rewrite, which uses compgen. It fix the issue with not displaying kernel modules with digits inside their names, which means that previously a loaded kernel module called "i915kms" would never be suggested to the user by Bash completions. Also, thanks to the replacement of sed with compgen, the user will no longer see errors like "RE error: parentheses not balanced" when requesting completion after typing "kldunload /".
There was a problem hiding this comment.
Thanks. In addition to the one inline comment, it would be really good to have some kind of better test possibilities for this. The only FreeBSD box I have convenient access to just outputs "kernel" from kldstat and my FreeBSD knowledge is very minimal, so there's not much I can say about this.
One very good thing would be to install a mock kldstat command, and to have it output something that is expected on a typical FreeBSD box. There's an example of this for showmount in test/lib/completions/mount.exp and test/fixtures/mount/. Would you be interested in taking a look at making the test suite use an approach like this for kldstat (and other FreeBSD specific commands of course, if you feel like it)?
| @@ -7,8 +7,8 @@ _kldunload() | |||
| local cur prev words cword | |||
| _init_completion || return | |||
|
|
|||
| COMPREPLY=( $( kldstat | command sed -ne \ | |||
| "s/^.*[[:blank:]]\{1,\}\($cur[a-z_]\{1,\}\).ko$/\1/p" ) ) | |||
| COMPREPLY=( $( compgen -W '$(kldstat)' -X 'kernel' -X '!*ko' -- "$cur" ) ) | |||
There was a problem hiding this comment.
The !*ko here should probably be !*.ko, no?
There was a problem hiding this comment.
It shouldn't make any difference but yeah, !*.ko is more robust.
|
Thank you for your response!
Yeah, if
I'll try to send some patches implementing those suggestions. I'll do it for the Thank you for your time! 😄 |
|
Merged with the |
There were some issues with the sed implementation of kldunload completions. This is a rewrite, which uses compgen.
It fix the issue with not displaying kernel modules with digits inside their names, which means that previously a loaded kernel module called "i915kms" would never be suggested to the user by Bash completions.
Also, thanks to the replacement of sed with compgen, the user will no longer see errors like "RE error: parentheses not balanced" when requesting completion after typing "kldunload /".
The patch was tested on FreeBSD 12.0-CURRENT. It should not cause any undesired behaviours. I also check related test files but it seemed to be unnecessary to modify them in this case.