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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[zsh] Addition of a completion script automatically generated #1034

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

buzuck
Copy link

@buzuck buzuck commented Aug 5, 2020

The problem

There is no auto-completion suggestions for ZSH.

Solution

Add auto-completion to ZSH, by generating that completion file from
data/actionmap/yunohost.yml.

This completion script makes use of the caching capabilities of ZSH to provide elements of completion that can take some time to generate, such as the list of the applications.

PR Status

  • Generation of the completion script done in a development environment
  • Tested on my YunoHost production instance, v3.x
  • This is the first time I tried creating a ZSH completion script. If anyone has some experience in this, any comment is welcome! 馃檪

There is also a few comments and todos in the headers of yunohost.yml and zsh_completion.py.

I tried to keep the code commented. I hope it is not overcomplicated, and that is quite readable... 馃槄

How to test

The completion file is created with:

python3 zsh_completion.py [--debug <OUTPUT_FILE>]

If no argument is given, the script tries to write the completion script into /usr/local/share/zsh/site-functions/_yunohost. The targeted directory has to be present in the variable $fpath for ZSH to find the completion script.

Install zsh with apt, enter the zsh shell and then run compinstall with the default values to enable completion.

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :
    • Items added to yunohost.yml
    • Overall architectural code choices for zsh_completion.py
    • Code style, naming, ...
    • Validate the caching mechanism of ZSH
    • Optional: Review the ZSH completion script

# cache is local to user
# - implement a zstyle switch, to change the cache validity period?

ZSH_COMPLETION_FILE = '/usr/local/share/zsh/site-functions/_yunohost'
Copy link
Member

@alexAubin alexAubin Aug 28, 2020

Choose a reason for hiding this comment

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

Hey there !

Small issue to fix for the debian packaging : that file won't be packaged. Instead, if we stick to something similar what's done for the bash completion : the output should go to some folder inside the yunohost/ tree
(for example bash completion goes into bash-completion.d/yunohost) then gotta add a rule inside debian/install to tell it the source/destination during the actual install of the .deb

Copy link
Author

@buzuck buzuck Sep 1, 2020

Choose a reason for hiding this comment

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

Thanks! I didn't see this Debian packaging aspect. I just pushed something in this regard.

@buzuck
Copy link
Author

buzuck commented Sep 1, 2020

While going through the Debian's packaging file, I saw Jinja2 is required
I've also seen that jinja is required by YunoHost. I refrained using it, as I wanted to have as little dependencies as possible. Jinja could definitely help generating this ZSH completion file, but I have no idea if this lib is available when the package is being build 馃槙

@alexAubin
Copy link
Member

alexAubin commented Sep 1, 2020

Ah yes indeed

What matters here is wether or not the lib is in Build-Depends :

https://github.com/YunoHost/yunohost/blob/dev/debian/control#L5

And you can see that it is ;) We already use it at build time ... not sure to remember what for, maybe some doc generation

@buzuck
Copy link
Author

buzuck commented Sep 1, 2020

Great! I'll be rewriting my Python code then 馃槂

And perchance, If anyone around has a knowledge of the ZSH completion framework, I would be interested to get a review on this topic, as this is my very first ZSH completion plugin 馃槄

@alexAubin alexAubin added this to the 4.1.x milestone Sep 5, 2020
@alexAubin alexAubin changed the base branch from buster-unstable to dev Sep 5, 2020
@zamentur zamentur modified the milestones: 4.1.x, 4.2.x Jan 3, 2021
@zamentur zamentur removed this from the 4.x milestone Jan 4, 2021
@zamentur zamentur added this to 4.x in Pending Jan 4, 2021
@kay0u
Copy link
Member

kay0u commented Oct 26, 2021

I was about to rewrite the bash_completion script to have something like this, so I think we could use your advanced work and move forward to have a jinja template for bash and zsh, and common python code to generate these files.

I guess we could first finish this PR, then I'll move to bash completion.

The work that remains to be done is to write the Jinja model, then adapt the code, right?

- uid
- cn
- mail
- maildrop
- loginShell
- homeDirectory
- mailuserquota
Copy link
Member

@kay0u kay0u Oct 26, 2021

Choose a reason for hiding this comment

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

Suggested change
- uid
- cn
- mail
- maildrop
- loginShell
- homeDirectory
- mailuserquota
- username
- fullname
- mail
- mail-alias
- mail-forward
- mailbox-quota
- groups
- shell
- home-path

autocomplete: &username
name: user
ynh_selector: user list
jq_selector: '.users|keys[]'
--purge:
Copy link
Member

@kay0u kay0u Oct 26, 2021

Choose a reason for hiding this comment

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

is it possible to display this arg in the "list of possible args" but not to autocomplete it when tabbing (this could be dangerous)?

pattern: *pattern_groupname
usernames:
help: User(s) to add in the group
nargs: "*"
metavar: USERNAME
extra:
autocomplete: *username
Copy link
Member

@kay0u kay0u Oct 26, 2021

Choose a reason for hiding this comment

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

is it possible to display only user not already it this group?

@@ -412,6 +476,7 @@ user:
username:
help: Username of the user
extra:
autocomplete: *username
Copy link
Member

@kay0u kay0u Oct 26, 2021

Choose a reason for hiding this comment

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

When i try to autocomplete this, i have:

$ yunohost user ssh add-key [tab]
_arguments:comparguments:325: invalid argument: 2

(Same when I put a valid username)

@@ -427,6 +492,7 @@ user:
username:
help: Username of the user
extra:
autocomplete: *username
Copy link
Member

@kay0u kay0u Oct 26, 2021

Choose a reason for hiding this comment

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

Same as user_ssh_keys_add

@@ -567,6 +647,7 @@ domain:
domain:
help: The domain for the web path (e.g. your.domain.tld)
extra:
autocomplete: *domains_list
pattern: *pattern_domain
path:
Copy link
Member

@kay0u kay0u Oct 26, 2021

Choose a reason for hiding this comment

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

Same:

$ yunohost domain url-available 
_arguments:comparguments:325: invalid argument: 2

@@ -757,6 +838,7 @@ app:
full: --installed
Copy link
Member

@kay0u kay0u Oct 26, 2021

Choose a reason for hiding this comment

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

yunohost app search 
_arguments:comparguments:325: invalid argument: 1
yunohost app manifest 
_arguments:comparguments:325: invalid argument: 1

@@ -819,6 +916,8 @@ app:
arguments:
app:
help: App to remove
extra:
autocomplete: *app_list
-p:
Copy link
Member

@kay0u kay0u Oct 26, 2021

Choose a reason for hiding this comment

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

maybe do not auto-complete if possible too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4 participants