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 shell completions for all versions #3012

Closed
wants to merge 5 commits into from

Conversation

hltdev8642
Copy link
Contributor

Created completion files by hand, paraphrasing description/value fields to make them a suitable length to be displayed in terminal (singleline).

Not sure exactly where these would be put within the repo, so I made a completions directory for the time being to place the files.

Hope this may be a helpful contribution, as I always wanted a completion file but couldn't find one or autogenerate it properly (so decided to make it by hand).

Best,
Jared (hltdev)

@rom1v
Copy link
Collaborator

rom1v commented Feb 10, 2022

Hi, thank you, there was even a feature request for that: #2930 😉

Remarks:

  • the 2 files look almost identical, could they be "merged" into one?
  • the format seems different from the files in /usr/share/bash-completion/completions (which btw don't provide a text description for each command).

@yangfl is there a specific format required by Debian?

@yangfl
Copy link
Contributor

yangfl commented Feb 10, 2022

AFAIK there's no such specification. But seems these completions are for zsh and, of course we can ship them into /usr/share/zsh/vendor-completions, this seems a bit less useful as Debian's default shell is bash.

(Disclaimer: I use zsh too.)

@hltdev8642
Copy link
Contributor Author

  • the 2 files look almost identical, could they be "merged" into one?

Correct. They are identical except for _scrcpy.exe vs _scrcpy , which would just determine which word would trigger completion in the terminal (I have scrcpy aliased to scrcpy.exe on WSL anyways, which results in scrcpy [tab] and scrcpy.exe [tab] both triggering the completion anyways.

So basically yes, you could potentially merge them but it would only trigger for either the scrcpy.exe or scrcpy command. FYI I generated the completion using the windows version (assuming that all the distributions have the same set of commands).

  • the format seems different from the files in /usr/share/bash-completion/completions (which btw don't provide a text description for each command).

as @yangfl said it is a zsh completion 😉 .
I can create a bash one too, shouldn't be too difficult. Could do that in the next few days and add it to the pull request 😄 👍

@rom1v
Copy link
Collaborator

rom1v commented Feb 14, 2022

you could potentially merge them but it would only trigger for either the scrcpy.exe or scrcpy command.

This is the format that you will use most often for the first line, but you can also use the same file for completing several different functions if you want. See here for more details.

https://github.com/zsh-users/zsh-completions/blob/3fb84ee9cc290adb38ac02c629f7fc026acfd2ff/zsh-completions-howto.org#telling-zsh-which-function-to-use-for-completing-a-command

(after a quick look, it's not clear to me how to, but it looks possible)

I can create a bash one too

That would be great 👍

Thank you :)

@hltdev8642
Copy link
Contributor Author

hltdev8642 commented Feb 15, 2022

@rom1v, I got the zsh one working in a single file thanks to that link you sent me , and have pushed the changes 👍 😄 👍.

@yangfl
I started working on a bash completion, which works but only completes the flags (w/o any description fields). Not too sure if having descriptions is even possible/needed for a bash completion anyways (so unless someone here knows I can look into that).
I can push that file as well if it will be good enough in that state, otherwise I will work on figuring out how to add the descriptions to the bash completion (and the zsh one should be all set now).

Best,
Jared

rom1v pushed a commit that referenced this pull request Feb 20, 2022
PR #3012 <#3012>

Signed-off-by: Romain Vimont <rom@rom1v.com>
@rom1v
Copy link
Collaborator

rom1v commented Feb 20, 2022

I squashed your commits into a single one, then applied the following changes:

  • install to $PREFIX/share/zsh/site-functions (it does not work if I install the script to /usr/local/share/zsh/vendor-completions)
  • add completion for new options implemented recently (-d, -e, --no-cleanup, --print-fps)
  • remove '*:filename:_files' (scrcpy never accepts an input filename)
  • remove . at the end of descriptions for consistency
  • remove execution flag for _scrcpy (chmod -x) (the completions scripts are not "executable")
  • add missing values for --lock-video-orientation
  • document possible values using the same pattern everywhere: (val1, val2, val3…)
  • fix some typos

The resulting commit is here: zsh_completion

Please review :)

@rom1v
Copy link
Collaborator

rom1v commented Feb 21, 2022

(FYI, I started working on a bash completion script yesterday, I will continue tonight or tomorrow.)

rom1v pushed a commit that referenced this pull request Feb 21, 2022
PR #3012 <#3012>

Signed-off-by: Romain Vimont <rom@rom1v.com>
rom1v pushed a commit that referenced this pull request Feb 22, 2022
PR #3012 <#3012>

Signed-off-by: Romain Vimont <rom@rom1v.com>
@rom1v
Copy link
Collaborator

rom1v commented Feb 22, 2022

I fixed the completion of parameters: zsh_completion.2

For example:

% scrcpy --lock-video-orientation=
Completing orientation
0         1         2         3         initial   unlocked

@rom1v
Copy link
Collaborator

rom1v commented Feb 22, 2022

I started working on a bash completion, which works but only completes the flags (w/o any description fields). Not too sure if having descriptions is even possible/needed for a bash completion anyways (so unless someone here knows I can look into that).
I can push that file as well if it will be good enough in that state, otherwise I will work on figuring out how to add the descriptions to the bash completion (and the zsh one should be all set now).

Oh, sorry, I missed the part where you actually started working on it (I thought you just planned to).

(FYI, I started working on a bash completion script yesterday, I will continue tonight or tomorrow.)

I finalized my script yesterday evening, I planned to open a PR after this one is merged, but I can share it here: bash_completion

Could you share a branch of your current bash script, please? Maybe there are possible improvements on both. Thank you.

@hltdev8642
Copy link
Contributor Author

I fixed the completion of parameters: zsh_completion.2

For example:

% scrcpy --lock-video-orientation=
Completing orientation
0         1         2         3         initial   unlocked

looks great, nice job! 👍 😄 👍

Could you share a branch of your current bash script, please? Maybe there are possible improvements on both. Thank you.

Sure, I just finished it, pretty basic but does work nonetheless!

Note that I don't have a ton of experience with branch-related tasks using git, so hopefully I did this correctly...
(if not, definitely let me know , as I am more then happy to learn something new👍)

[bash-completion]

rom1v pushed a commit that referenced this pull request Feb 22, 2022
PR #3012 <#3012>

Signed-off-by: Romain Vimont <rom@rom1v.com>
rom1v added a commit that referenced this pull request Feb 22, 2022
@rom1v
Copy link
Collaborator

rom1v commented Feb 22, 2022

looks great, nice job! 👍 😄 👍

Thank you, merged 👍 2695378

Sure, I just finished it, pretty basic but does work nonetheless!

Thank you. I took the same example as yours initially, but it did not work correctly both with and without =. For example:

  • --lock-video-orientation without argument
  • --lock-video-orientation=1
  • --lock-video-orientation 1

I submitted a PR for my bash completion script: #3048.

I will publish a new release soon (v1.23), so I will merge it just before.

@rom1v rom1v closed this Feb 22, 2022
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

3 participants