Add first version of zsh completion

[haraldkubota]

Adding a zsh completion script. Tested extensively for the options I use a lot.
I put the file into contrib/zsh_completion instead of src/ where the bash_completion is as makes more sense to me. Feel free to move it around if you prefer. Or move the back_completion into contrib/

[rusty-snake]

If we put it in src, we could run it through gcc and remove options which are disable at compile time.

[haraldkubota]

That's an interesting idea. Put it through cpp as part of "make" in src/firejail and plenty #ifdef in a _firejail.cpp which then generates a _firejail? Can do.
Or do you have another method in mind?

[rusty-snake]

Yes, like this. Except that I would use src/zsh_completion and _firejail.in. Then we could use #ifdef HAVE_X11, #ifdef HAVE_DBUSPROXY and so on.

[rusty-snake]

Sorting all this would simplify future maintenance. For example --seccomp, --seccomp.blocksecondary and --seccomp.drop are at three different places.

---

Aside notes:

• we should also pre-process bash-completions
• some of our --help descriptions are wrong/outdated/incomplete or can be improved in other ways.

[rusty-snake]

If I type firejail --profile= and press tab, only directories in the current working directory are shown (no files from cwd; files from cwd will be complete after you begin to enter them) and $HOME/.config/firejail/*.profile.

It would be nice to show/complete files from cwd and profile names for firejail --profile=<PROFILE-NAME> foo (e.g. firejail --profile=shellcheck foo) too. Maybe we can reuse a modified version of _profiles and _all_profiles https://github.com/rusty-snake/fjp/blob/fa79d89504d425906a84590758137d1fa12b58fc/build.rs#L28-L57.

[haraldkubota]

I see what you mean. Let me look into that.

[haraldkubota]

What's this '(--foo)'{--foo=,foo=} syntax, I could find anything about it?

The (--foo) means: don't show this option again once a --foo option was given.
The {--foo=,--foo=} is usually for short and long options, e.g. {-f,--foo}, but since firejail does not have those, it's twice the same. I cannot remove it and I cannot remove the 2nd argument either.
The docs at http:https://zsh.sourceforge.net/Guide/zshguide06.html (section 6.8.2) write correctly:

The function _arguments has been described as having the syntax from hell', but with the arguments already laid out in front of you it doesn't look so bad. The are three types of argument: options to _arguments itself, arguments saying how to handle options to the command (i.e. p4 diff'), and arguments saying how to handle normal arguments to the command.

[haraldkubota]

Using cpp for shell scripts turns out to be a bad idea and I had to work around small issues, but it's done and it works. Using unifdef would have been better, but that's another build dependency.
I have never used autoconf before and m4 never got any love from me, so please check my small modification there.

[rusty-snake]

I have never used autoconf before and m4 never got any love from me, so please check my small modification there.

I've less experience with autotools too, so this need to do some one else.

Using cpp for shell scripts turns out to be a bad idea and I had to work around small issues, but it's done and it works. Using unifdef would have been better, but that's another build dependency.

If it works now, very thing is good. But if we have future trouble with it, we may just use preproc.awk for it.

---

What's this '(--foo)'{--foo=,foo=} syntax, I could find anything about it?

[haraldkubota]

--profile is much better now. I did not know you can simply list the profile name without .profile. In fact, adding .profile to a system profile causes an error.
What you cannot do now is use autocompleter for arbitrary profiles. In your CWD it'll work if you type --profile=./, but otherwise it'll "only" show profiles in ~/.config/firejail/ and $PREFIX/etc/firejail/
Which is 100% of my use-cases and I cannot imagine many people doing something very different. They can always type the actual path if needed.

[rusty-snake]

Now I tested this, sorry for the delay. IMO this can be merged once the points below are solved:

• src/zsh_completion/README.md can be removed (we install it)
• --profile= can only work with name of .profile but not .local or .inc
• _PREFIX_ does not fork when configured with --prefix=/usr, sysconfdir should be used instead. (sysconfdir=@sysconfdir@ is in common.mk)
• (optional) rebase and squash

[haraldkubota]

Thanks for the review! All done. Including the git rebase/squash.

[rusty-snake]

Including the git rebase/squash.

It's squashed, that's right. However, a37ffc3 has all the other commits as parents.
If you rebase against 6ca31ec (which seems to be the news commit at https://github.com/haraldkubota/firejail/commits/master also here in master) and drop every commit except this squashed one and then force-push it, it should be clean.

git rebase -i 6ca31ec520c3b304b9bbae9190262f08eadfa8e1
# Replace pick with drop for every commit except a37ffc3
git push --force

Aside: Using feature-branches is often easier to rebase and allows to have multiple open PRs.

[rusty-snake]

There a lot more that can be done in the future like suggesting binaries for private-bin or no suggesting mutually exclusive options such as --profile and --noprofile. I will definitely add more features.

[haraldkubota]

git rebase -i 6ca31ec520c3b304b9bbae9190262f08eadfa8e1
# Replace pick with drop for every commit except a37ffc3
git push --force

Thanks a lot for this. While I use git for a long time, this is the first time I rebase/squash. While I eventually would have figured it out, it's nice to see a working solution.
Done.

Aside: Using feature-branches is often easier to rebase and allows to have multiple open PRs.

Live and learn 👍

[haraldkubota]

There a lot more that can be done in the future

From experience: once there's something it's much easier to build on that and extend it. And that was part of my plan: have zsh completion basically working so other people can easily add one new feature or two.

[nrdxp]

why was this closed?

[haraldkubota]

why was this closed?

Hmm...stupidity by the user? Me in this case?

[rusty-snake]

Thanks for this contribution!