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

hyprctl: Add shell completions #5404

Merged
merged 14 commits into from
Apr 4, 2024
Merged

hyprctl: Add shell completions #5404

merged 14 commits into from
Apr 4, 2024

Conversation

LOSEARDES77
Copy link
Contributor

@LOSEARDES77 LOSEARDES77 commented Apr 3, 2024

Describe your PR, what does it fix/add?

Add shell completions for bash and fish for the hyprctl command

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

Have only tested in bash and fish shell

Is it ready for merging, or does it need work?

Its still WIP
Its my first time experimenting with completions so i don`t know if this is the correct way of doing it but this just works for me

Fish:
image

Bash:
image

@htomeht
Copy link

htomeht commented Apr 3, 2024

It would appear that this disregards nesting. It just gives the completion on the first level.

hyprctl dispatch clients
activewindow getoption output version
activeworkspace globalshortcuts plugin workspacerules
binds hyprpaper reload workspaces
clients instances rollinglog -j
configerrors keyword setcursor -r
cursorpos kill seterror --batch
decorations layers setprop --help
devices layouts splash --instance
dismissnotify monitors switchxkblayout
dispatch notify systeminfo

@LOSEARDES77 LOSEARDES77 changed the title hyprctl: Add shell completions [WIP] hyprctl: Add shell completions Apr 3, 2024
@LOSEARDES77
Copy link
Contributor Author

It would appear that this disregards nesting. It just gives the completion on the first level.

hyprctl dispatch clients
activewindow getoption output version
activeworkspace globalshortcuts plugin workspacerules
binds hyprpaper reload workspaces
clients instances rollinglog -j
configerrors keyword setcursor -r
cursorpos kill seterror --batch
decorations layers setprop --help
devices layouts splash --instance
dismissnotify monitors switchxkblayout
dispatch notify systeminfo

i see what you mean,
I'll make it work i progress

@LOSEARDES77
Copy link
Contributor Author

I think its ready for revision

@LOSEARDES77 LOSEARDES77 changed the title [WIP] hyprctl: Add shell completions hyprctl: Add shell completions Apr 3, 2024
@vaxerski vaxerski requested a review from fufexan April 3, 2024 19:48
@LOSEARDES77
Copy link
Contributor Author

There is a issue when doing sudo make install the ~ is /root, ${HOME} is /root and ${USER} is root too.
Because of this the completions files source to the root user.
I don't know how to address this

@fufexan
Copy link
Member

fufexan commented Apr 3, 2024

Sourcing shouldn't be necessary. Shells will automatically load the completions from their default directories.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@LOSEARDES77 LOSEARDES77 requested a review from fufexan April 3, 2024 21:45
Copy link
Member

@fufexan fufexan left a comment

Choose a reason for hiding this comment

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

Tested functionality in all the shells and it seems to be working fine.

I've also added the install instructions to meson.

LGTM. Thanks for the contribution!

@fufexan fufexan merged commit 1b43cd5 into hyprwm:main Apr 4, 2024
9 checks passed

<MONITORS> ::= {{{ hyprctl monitors | grep Monitor | awk '{ print $2 }' }}}

<KEYWORDS> ::= {{{ hyprctl devices | sed -n '/Keyboard at/{n; s/^\s\+//; p}' }}}
Copy link
Contributor

Choose a reason for hiding this comment

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

this supposed to be <KEYBOARDS> ?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it. I'll push a fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

it also means that actual <KEYWORDS> array is missing, referenced on line 71

@fufexan
Copy link
Member

fufexan commented Apr 4, 2024

I was too hasty in merging this.

@LOSEARDES77 it looks like the KEYWORDS definition does not exist. Were you planning on adding it?

@LOSEARDES77
Copy link
Contributor Author

LOSEARDES77 commented Apr 4, 2024

I forgot to add the keywords

@LOSEARDES77
Copy link
Contributor Author

I'll work on hyprpm completions today and add the missing keywords

@LOSEARDES77
Copy link
Contributor Author

Can't find a way to retrieve aviable keywords

@solopasha
Copy link
Contributor

https://github.com/scop/bash-completion/blob/master/README.md
bash completions should be installed in

❯ pkg-config --variable=completionsdir bash-completion
/usr/share/bash-completion/completions

for fish:
from https://fishshell.com/docs/current/completions.html:

A directory for third-party software vendors to ship their own completions for their software, usually /usr/share/fish/vendor_completions.d;

@LOSEARDES77
Copy link
Contributor Author

I see ill applt suggested changes to #5423

lisuke pushed a commit to lisuke/Hyprland that referenced this pull request Apr 15, 2024
@Ordoviz
Copy link
Contributor

Ordoviz commented Apr 16, 2024

This could be improved a little further by taking inspiration from #3535

@fufexan
Copy link
Member

fufexan commented Apr 17, 2024

@Ordoviz I completely missed that PR, apologies. Can you rebase it and make whatever changes you see fit?

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.

6 participants