Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

completions/insmod: Add support for insmod command #10410

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

FirstLoveLife
Copy link

Description

This commit introduces a new completion script for the insmod command, enabling Fish shell users to autocomplete file for .ko (kernel module) files in the current directory.

This commit introduces a new completion script for the `insmod` command,
enabling Fish shell users to autocomplete file for `.ko` (kernel
module) files in the current directory.
function _insmod_complete
# List all .ko files in the current directory for completion.
for file in (ls *.ko)
echo $file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This for-loop is broken in a number of ways:

  1. ls can take options
  2. ls * is duplicating work - we already know that the given files exist once the glob matches
  3. It only works in the current directory and won't complete paths (like e.g. "/lib/modules/foo/", which you very likely want to do)
  4. The entire loop is unnecessary, you can just do printf '%s\n' *.ko

But mostly, this is duplicating __fish_complete_suffix .ko. Just use that, remove the entire function.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi faho, thanks for your quick reply.

It only works in the current directory and won't complete paths (like e.g. "/lib/modules/foo/", which you very likely want to do)

IMO "/lib/modules/foo/" can be left for modprobe, insmod is more commonly used to insert a ko in the local directory. Including /lib/modules for insmod seems somewhat overhead(at least to me).

But mostly, this is duplicating __fish_complete_suffix .ko. Just use that, remove the entire function.

Thanks for your tip! But I encounter an issue:
complete -c insmod -a "(__fish_complete_suffix .ko)" -f

This doesn't work somehow and TAB still lists all files, could you please provide some insights? Thanks.

Copy link
Member

@faho faho Apr 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO "/lib/modules/foo/" can be left for modprobe, insmod is more commonly used to insert a ko in the local directory. Including /lib/modules for insmod seems somewhat overhead(at least to me).

I see no reason to not support directories.

This doesn't work somehow and TAB still lists all files, could you please provide some insights? Thanks.

__fish_complete_suffix will complete all files, but leave matching ones at the top.

This is because the suffix is not actually authoritative - you can name files whatever you want.

This is a very common problem when people try to complete e.g. ".txt" files, and then notice that they have a lot of text files with another or no suffix. For modules specifically, you may find ".ko" files, but they can also be compressed - ".ko.gz".

What you want is

complete -c insmod -a "(__fish_complete_suffix .ko)" -kf

where the "-k" tells "complete" to not sort the completions but keep the order they're in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants