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

Client will install bash completion file. #213

Closed
wants to merge 1 commit into from

Conversation

zkl94
Copy link
Collaborator

@zkl94 zkl94 commented Nov 13, 2015

Add bash completion file to package and document python-argcomplete.

JIRA: PDC-1161

@zkl94 zkl94 force-pushed the PDC-1161_add_pdc_bash_completion branch 3 times, most recently from 11d000c to 9981c98 Compare November 13, 2015 05:42
@zkl94
Copy link
Collaborator Author

zkl94 commented Nov 13, 2015

@lubomir @xychu
I think this one needs some discussion:

  1. if we choose to add bash completion file in /etc/bash_completion.d/ and not ask users to do the configuration manually, we will only have bash support. And if we only want bash support, we can use the global_completion to further simplify the process by simply adding PYTHON_ARGCOMPLETE_OK mark to executables rather than add the bash completion file to user system.
  2. if we want to support both bash and zsh (i haven't tested on other shells), we will have to do the configuration manually according to argcomplete issue 10 and global_completion.

So do we need to support bash as well as zsh or just support bash and drop zsh?

@erichuanggit
Copy link
Collaborator

I would say let's do one then the other if the requirement is required from customer, but we need to NOTE it in the document.

@lubomir
Copy link
Member

lubomir commented Nov 13, 2015

It would be nice to support more shells, but I don't want zsh blocking bash.

These changes look fine to me. However, the main thing must change in the packaging: assuming user has python-argcomplete installed already and installs the client package, the completion must work without any manual intervention.

I don't see the advantage of global support. It still requires us to install a completion file, and has the potential of interacting with other python programs, which I think is not desirable.

@zkl94
Copy link
Collaborator Author

zkl94 commented Nov 13, 2015

@lubomir global support doesn't require completion file, we just have to add PYTHON_ARGCOMPLETE_OK before executables in order to make it work. So there is no eval or completion file needed, everything is contained in in the script which we want to have auto completion capability.

@zkl94
Copy link
Collaborator Author

zkl94 commented Nov 13, 2015

@lubomir I see pip has the function we want: if we press <tab> after typing pip install, pip gets stuck for a while to fetch package index cache from the web. This trick may work for pdc where we can ask users whether to install python-argcomplete if he presses <tab> after pdc without python-argcomplete being installed.

@lubomir
Copy link
Member

lubomir commented Nov 13, 2015

@lao605 Does that work completely automatically or did you have to run activate-global-python-argcomplete at some point in the past?

@zkl94
Copy link
Collaborator Author

zkl94 commented Nov 13, 2015

@lubomir yes, users will have to run activate-global-python-argcomplete first where python-argcomplete.sh will be installed. Although these two means of implementation all have a file installed (one is pdc and another is python-argcomplete.sh), global support is cleaner, that is, we don't have to install completion files for future executables (if there are).

@lubomir
Copy link
Member

lubomir commented Nov 13, 2015

It is a problem that users have to run a command. How are they going to know they should do it?

Apart from knowing that running the command is necessary, is is also required to have root access (to put the file under /etc) or further modify the configuration (sourcing the file from ~/.bashrc). None of these options is user-friendly.

@zkl94
Copy link
Collaborator Author

zkl94 commented Nov 13, 2015

@lubomir As for user-friendliness, the implementation you mention is better: user will just have to install python-argcomplete to get auto-completion work. Although we can put some notes about that command in the doc, it seems unnecessary if we can skip that (maybe as less commands as possible). User-friendliness is more important than extensibility at this point, am I correct?

@zkl94
Copy link
Collaborator Author

zkl94 commented Nov 16, 2015

I think of a third solution where we can put python-argcomplete.sh file in bash completion scripts directory during package installation rather than put completion file of pdc specifically in bash completion scripts directory. So I think there are totally three plans we have now:

  1. put pdc completion file (the result of eval "$(register-python-argcomplete pdc)") in bash completion scripts directory (which is always /etc/bash_completion.d/ for CentOS/RHEL)
  2. let user run eval "$(register-python-argcomplete pdc)" when they need it
  3. put python-argcomplete.sh (which is what activate-global-python-argcomplete puts in bash completion scripts directory); and developers should leave a PYTHON_ARGCOMPLETE_OK at the head of executables ready for auto-completion.

I have taken three factors in account: user-friendliness, extensibility and support for shells. So here is my analysis:

user-friendliness

by saying user-friendliness, I mean the steps users needed to do to make auto-completion work for pdc.

  1. for plan 1, there will be two steps users (rather than one): install bash-completion using yum and install python-argcomplete using yum or pip. bash-completion functionality is not automatically enabled for CentOS/RHEL and must be enabled manually.
  2. for plan 2, there will be two steps for users: install python-argcomplete using yum or pip and put the eval command argcomplete suggests in ~/.bashrc
  3. for plan 3, there will be two steps for users which is the same with the first plan.

Although they all require 2 steps to make it work, plan 1 and plan 3 outweight plan 2 because in the plan 2 editing of ~/.bashrc file is needed. plan 1 and plan 3 wins.

extensibility

  1. plan 1: for developers, when a new executable is to be added to pdc (if any), he will have to add a completion file for that exec and edit the spec file. This produces completion files scattered in pdc_client; for users, there is nothing to do when pdc upgrades.
  2. plan 2: for developers, there is nothing to do other than write the new exec; for users, they have to manually add that to ~/.bashrc
  3. plan 3: for developers ,there is nothing needed to do other than add a PYTHON_ARGCOMPLETE_OK at the top of the exec; for dev, there is nothing needed to do.

So, as for extensibility, plan 3 wins.

support for shells

  1. plan 1 only supports bash
  2. plan 2 support bash and zsh although there seems to be a different mechanism for zsh to do auto-complete
  3. plan 3 only support bash

so, as for support for shells, plan 2 wins. But shells support is not of primary concern.

plan 3 is the same with plan 1 in user-friendliness and beat plan 1 and plan 2 in extensibility. I think it is a workable plan.

@xychu
Copy link
Contributor

xychu commented Nov 16, 2015

Good news is that pdc-client repo is ready,
bad news is that this pull request is not included, sorry for the inconvenience.
Please create it again against the new repo.

@zkl94 zkl94 closed this Nov 18, 2015
@zkl94 zkl94 deleted the PDC-1161_add_pdc_bash_completion branch November 18, 2015 05:36
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

4 participants