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

Add completions for loadkeys #9312

Merged
merged 1 commit into from
Nov 1, 2022
Merged

Add completions for loadkeys #9312

merged 1 commit into from
Nov 1, 2022

Conversation

llenck
Copy link
Contributor

@llenck llenck commented Oct 29, 2022

Description

Adds completion for loadkeys.

TODOs:

  • n/a Changes to fish usage are reflected in user documentation/manpages.
  • n/a Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@@ -0,0 +1,15 @@
complete -fc loadkeys -a "(find /usr/share/kbd/keymaps/ -type f | string replace -rf '.*/(.*).map.gz' '\$1')"

complete -c loadkeys -s C -l console -d "Console device to use" -x -a "(command ls /dev/tty*)"
Copy link
Member

Choose a reason for hiding this comment

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

Use path filter --type=char /dev/tty* instead of command ls (which might have portability issues, will print directories, requires starting another command, and will print an error if no files match).

@@ -0,0 +1,15 @@
complete -fc loadkeys -a "(find /usr/share/kbd/keymaps/ -type f | string replace -rf '.*/(.*).map.gz' '\$1')"
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure these need to be .gz, so I'd probably make that second extension optional, maybe even up to

string replace -rf '.*/(.*).map(\..*)' '\$1'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, and I also just found out that /usr/share/kbd/keymaps isn't as standardized as the manual I read made it seem. I will update this later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the PR to work with all paths I could find for different distributions, and all extensions that loadkeys looks for, according to its source code.

# systemd, which we shouldn't depend on.

for d in '/usr/share/kbd/keymaps' '/usr/share/keymaps' '/usr/lib/kbd/keymaps' '/lib/kbd/keymaps' '/usr/src/linux/drivers'
[ -d $d ] && find $d -type f | string replace -rf '.*/(.*)\.k?map(|\.gz|\.bz2|\.xz|\.zst)$' '$1'
Copy link
Member

Choose a reason for hiding this comment

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

This can be simplified somewhat:

set -l dirs /usr/share/kbd/keymaps /usr/share/keymaps /usr/lib/kbd/keymaps /lib/kbd/keymaps /usr/src/linux/drivers
path filter -f $dirs/** | string replace -rf '.*/(.*)\.k?map(|\.gz|\.bz2|\.xz|\.zst)$' '$1'

No need to run find five times or test for the existence of the directories separately. You can just glob in them and keep the files.


Another thing: I wouldn't want to hardcode the compression extension here - I'd just look for a \.k?map and throw away any extension after that, so:

string replace -rf '.*/(.*)\.k?map(|\..*)$' '$1'

Now, technically this could be mis-matching something if there was ever a file with a different extension there - foo.map.this_is_not_a_map. However, I would call that less likely than someone using a compression algorithm you don't know here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I doubt supported compression schemes are going to change soon, and there are a bunch of weird extensions in /usr/src/linux, though I guess even if a false positive triggering file was added, it wouldn't be that harmful; I'll apply that.

Also sorry, I didn't know path had special behaviour for failed wildcard matches, so I didn't want to use that, as e.g. Ubuntu's loadkeys comes without any keymaps. I'll apply that as well.

@faho faho added this to the fish 3.6.0 milestone Nov 1, 2022
@faho faho merged commit c5a026c into fish-shell:master Nov 1, 2022
@faho
Copy link
Member

faho commented Nov 1, 2022

Merged, thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants