-
-
Notifications
You must be signed in to change notification settings - Fork 134
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
Register completion for multiple commands #246
Conversation
argcomplete/shellintegration.py
Outdated
@@ -55,8 +55,12 @@ def shellcode(executable, use_defaults=True, shell='bash', complete_arguments=No | |||
complete_options = " ".join(complete_arguments) | |||
|
|||
if shell == 'bash': | |||
code = bashcode | |||
quoted_executables = ['"%s"' % i for i in executables] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should use shlex.quote
for this (it's small enough that you could copy and paste it for use on Python 2).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chrisdiamand, please rebase on master. That should make the OS X test pass.
Also, please add a test where multiple executables are passed (["prog", "prog"]
?)
Other than that and @evanunderscore's comment, LGTM
Codecov Report
@@ Coverage Diff @@
## master #246 +/- ##
==========================================
+ Coverage 87.88% 88.03% +0.14%
==========================================
Files 6 6
Lines 652 660 +8
==========================================
+ Hits 573 581 +8
Misses 79 79
Continue to review full report at Codecov.
|
Shell startup files may contain lots of 'register-python-argcomplete' calls, for as many scripts as require it. However, this can have a significant impact on shell startup time, because each one requires starting a Python interpreter and invoking 'complete'. Optimise this by allowing 'register-python-argcomplete' to accept multiple commands. In Bash, the whole list of completable commands can be passed to a single 'complete' call. Tcsh requires a separate 'complete' call for each command, but we can at least avoid the overhead of starting Python each time. On my machine, the time to register ~10 completed scripts decreases from about 0.4s to 0.05s with this change.
Hi - thanks for the feedback. I think I've addressed that in the latest version, please let me know if I've misunderstood something. Cheers! |
Sorry about the delay in merging this. |
Thanks! |
Regression introduced by c37cd68, which bumped argcompletion, which included an undocumented breaking change (kislyuk/argcomplete#246) that lead to generating `d m a k e` instead of `dmake` in the final bash script, breaking the completion (and maybe other things).
Regression introduced by c37cd68, which bumped argcompletion, which included an undocumented breaking change (kislyuk/argcomplete#246) that lead to generating `d m a k e` instead of `dmake` in the final bash script, breaking the completion (and maybe other things).
Shell startup files may contain lots of 'register-python-argcomplete'
calls, for as many scripts as require it.
However, this can have a significant impact on shell startup time,
because each one requires starting a Python interpreter and invoking
'complete'.
Optimise this by allowing 'register-python-argcomplete' to accept
multiple commands. In Bash, the whole list of completable commands
can be passed to a single 'complete' call. Tcsh requires a separate
'complete' call for each command, but we can at least avoid the
overhead of starting Python each time.
On my machine, the time to register ~10 completed scripts decreases
from about 0.4s to 0.05s with this change.