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.

Sign up for GitHub

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

Jump to bottom

completions/insmod: Add support for insmod command #10410

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

completions/insmod: Add support for insmod command #10410

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions share/completions/insmod.fish
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
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 •
edited

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.

end
end

complete -c insmod -a '(_insmod_complete)' -f