Improve argument parser + implement bash completion

[Vulcalien]

Closes #495

First, I had to study how bash completion works. To do so, I simply studied the scripts used by other applications.
Then, I used sed and regex in order to scrape the options from terminator --help. (I might have nightmares about regex...)

In order to make the completion work, I put a symlink to the new file in
/usr/share/bash-completion/completions named terminator.
I think this is something the package maintainers have to care about.

Sadly, each shell uses a different type of completion script. This means that this one in particular will only work on bash.

In order to get the options, terminator --help has to be run, so there is a very little overhead because python has to start.

Edit: the folder where the scripts are put might be different depending on the distro. On Debian it's the one I wrote.

[mattrose]

Thank you so much for this. One other thing that the user mentioned was autocomplete on profiles and layouts, so -p would autocomplete profiles and -l would autocomplete layouts. To do this we would have to dig into the internals a little bit more to get the list of profiles and layouts. I can take a stab at it if you don't want to.

[Vulcalien]

Oh ok, that shouldn't be too difficult. It's just a matter of creating an option like "--list-profiles" and parse it on the bash file. I can work on this tomorrow, it's not a problem.

[mattrose]

That would be great, thanks again.

[abrzozowski]

I see that you are using a native completion command from bash. I am suggesting using argcomplete its compatible with argparse. Apart from bash completion there is also limited support for zsh, fish, and tcsh available.

[Vulcalien]

Reading the code, we seem to still use optparse, a deprecated library.
Using a command completion directly inside Terminator could be useful, and easier to implement for every shell.
Anyways, I'll try to migrate to the new argparse library and see if it's easier to use argcomplete.

Update: it's... quite a lot of work. optionparse.py is completely based on optparse. I honestly would even suggest a complete rewrite of it.

[Vulcalien]

Ok another thing is done. The code I modified was mostly 12 years old. The new library is overall better, easier to use, does many things automatically and so on.

[mattrose]

I haven't merged this yet because I have a few tweaks to it. If it's OK with you I'll do a PR against your branch with them sometime soon.

[Vulcalien]

The translations files should be updated (because of --list-profiles and --list-layouts). Is there a way to automatically do it, or do I have to manually edit the them?

Also, I haven't implemented the profile and layouts completion yet, because I don't know if we should use argcomplete or plain bash.

[abrzozowski]

The argcomplete is more generic (limited support for zsh, fish, and tcsh) than the plain bash-completion. In addition, the argcomplete can be a little bit slower than bash-completion.

I have had experience in both so I can support you if you need something to talk things through.

[Vulcalien]

My concern is about using a non-standard module. I don't really know how these are handheld here.

[mattrose]

Unless we absolutely need a 3rd party library, I would not merge it in. I have a pretty big aversion to 3rd party python modules in general.

[abrzozowski]

So, in case, if we wouldn't like to use the argcomplete which isn't python's standard module, the choice is one: plain bash-completion.

We have to remember about add installing completion/bash to setup.py

[mattrose]

Since the issue specifies bash completions, let's just leave it at that. I'm still on the fence about putting the completions in as part of a standard install through setup.py, as I'm not sure there is a standard location across all of the platforms that terminator currently runs on where we can place that file. If anyone knows one I'd be happy to include, but for now, let's not worry about that.

[abrzozowski]

Ok, I understand your worries, but I presume that if the completer will be not installed, no one will notice that it exists. Maybe install completions only when Bash shell is detected on the current system?

I feel that https://github.com/scop/bash-completion/blob/master/README.md and https://ddev.readthedocs.io/en/v1.16.7/users/shell-completion/ are good references of a location for completers on UNIX shells.

[mattrose]

I would agree with you in general, but in our case, I'm one of the Fedora maintainers, and Markus is one of the debian maintainers, so between us most linux distros will pick up any packaging changes we make. I'll take a look at the docs and see if we can shoehorn it into setup.py somehow.

[mattrose]

@Vulcalien is this ready to go from your end, or are there more changes you have planned?

[Vulcalien]

I'm almost done. The only things remaining are the profile and layout completions (--profile=..., --layout=...) and I think the translation for the help messages of --list-profiles and --list-layouts.
I don't know how the translation files work. I think I'll write the missing completions tomorrow.

Edit: there is an update_all_po.sh script in po. I'll give a look at that, maybe it's just as easy as running that with the right stuff installed.

[Vulcalien]

There we go, translation files done. I haven't tested them, but I just run the automatic commands, so I don't expect anything to break.

[abrzozowski]

How is it going on with this PR? Do you need some hands to have work done?

[mattrose]

@Vulcalien is this ready to go now?

[Vulcalien]

The completion for --profile and --layout is still missing. I've recently been thinking of a solution (maybe make the output of --list-profiles and --list-layouts more bash-friendly).
However, the completion works for the generic arguments. Right now I have less time than before, but eventually I'll work on this.

I think it's ready to get merged and in a second moment we can work on this again.

[abrzozowski]

I've pushed improved bash-completion abrzozowski@fe3e6f0 which supports --profile and --layout arguments as well as improves completion for other arguments.
The previous completion lists strange options like [-d

<$:~/dev/.../completion (feature/add_bash_completion)$ terminator 
-b                   --debug              --fullscreen         --help               -l                   --maximise           --role               --unhide             
--borderless         --debug-classes      -g                   --hidden             --layout             --maximize           -s                   usage:               
--command            --debug-methods      --geometry           -i                   --list-layouts       --new-tab            --select-layout      -v                   
--config             [--debug-methods     [--geometry          [-i                  --list-profiles      --no-dbus            -T                   --version            
--config-json        -e                   -h                   --icon               [--list-profiles     -p                   terminator           --working-directory  
-d                   --execute            [-h                  -j                   -m                   --profile            --title              -x                   
[-d                  -f                   -H                   [-j                  -M                   -r                   -u   

Now it's

<$:~/dev/.../completion (feature/add_bash_completion)$ terminator --
--borderless         --debug              --fullscreen         --icon               --maximise           --profile            --unhide             
--command            --debug-classes      --geometry           --layout             --maximize           --role               --version            
--config             --debug-methods      --help               --list-layouts       --new-tab            --select-layout      --working-directory  
--config-json        --execute            --hidden             --list-profiles      --no-dbus            --title    

You're welcome to use this commit

The last thing to consider is installing completion to the system

[Vulcalien]

I'm testing this out. Thanks for making me notice the [-x thing and most importantly, write the regexes.

The new regex for the generic completion makes the options with only one dash not appear, for example -g (with is not the same as --geometry, so it needs to be shown)

I'm modifying the regex to see if I can make it work, thanks for help!

Edit: another problem, harder to solve is the profiles/layouts with a quote in the name. Eg. test''' is not the same as test. And we can't even assume that the name will be quoted using single quotes, because python prints the profile test' as "test'".

On a related note, creating a profile with both single and double quotes will fail silently. I think we should consider not allowing users to create profiles/layouts with quotes.

Update: I changed the output of --list-profiles and --list-layouts to a more readable one (both for humans and bash).
The completion for profiles/layouts is implemented, but doesn't really work if one of them has quotes or spaces in the name.
I'll try to fix this.

[Vulcalien]

Since this PR has been unchanged for 2 months, I think it's a good idea to merge the feature, even if it's partially implemented. Also, there is an under the hood improvement, that would be a good PR on its own.

Summary of the changes:

• Replace deprecated optparse module with argparse
• Add --list-profiles and --list-layouts options
• Update translation files
• Implement bash completion for options

Missing feature:

• Bash completion for --profile and --layout

Here is an attempt at implementing the missing feature:

case $prev in
    --profile | -p)
        COMPREPLY=($(compgen -W "$($1 --list-profiles)"\
        -- "$cur"))
        return
        ;;
    --layout | -l)
        COMPREPLY=($(compgen -W "$($1 --list-layouts)"\
        -- "$cur"))
        return
        ;;
esac

It doesn't work for profiles/layouts with whitespaces or quotes in the name (e.g. New Layout)

[mattrose]

You're right, this should go in. I'll work on getting the completions working with the spaces as well. I just need to remind myself to do it next time I look at the code.