Bug 475229
Summary: | Implement catch-all programmable completion handler | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Behdad Esfahbod <behdad> | ||||||||||
Component: | bash | Assignee: | Roman Rakus <rrakus> | ||||||||||
Status: | CLOSED UPSTREAM | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||||||
Severity: | medium | Docs Contact: | |||||||||||
Priority: | low | ||||||||||||
Version: | rawhide | CC: | fred, fvulto, guillomovitch, rhughes, rrakus, rstrode, siegfried, tsmetana, twaugh, ville.skytta | ||||||||||
Target Milestone: | --- | ||||||||||||
Target Release: | --- | ||||||||||||
Hardware: | All | ||||||||||||
OS: | Linux | ||||||||||||
Whiteboard: | |||||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||||
Doc Text: | Story Points: | --- | |||||||||||
Clone Of: | Environment: | ||||||||||||
Last Closed: | 2010-04-27 13:07:52 UTC | Type: | --- | ||||||||||
Regression: | --- | Mount Type: | --- | ||||||||||
Documentation: | --- | CRM: | |||||||||||
Verified Versions: | Category: | --- | |||||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||||
Embargoed: | |||||||||||||
Attachments: |
|
Description
Behdad Esfahbod
2008-12-08 15:50:31 UTC
Created attachment 326136 [details]
The patch
Created attachment 326142 [details]
Updated patch
The old one didn't correctly do the retry phase. This one does.
Created attachment 326147 [details]
One last time today
Ok, cleaned up retries. Now we retry from the beginning of the sequence. Stop retrying if none of the three cases had changed handlers since last round.
So if I understand it well: 1) completions are loaded onwork as needed? 2) This change speedup system startup, but slowdown bash completion itself. Do you agree? I don't see any other effects. Will be startup speedup notable? Or slowdown will be much higher? Thanks Roman for the reply. > So if I understand it well: > 1) completions are loaded onwork as needed? Yes. > 2) This change speedup system startup, but slowdown bash completion itself. Do > you agree? It doesn't affect system startup as bash-completion only loads on interactive shells. However, interactive shell startup becomes more than twice faster. Currently with bash-completion installed: behdad:~ 0$ time bash -c : real 0m0.006s user 0m0.002s sys 0m0.004s behdad:~ 0$ time bash -i -c : real 0m1.247s user 0m0.722s sys 0m0.135s If I remove bash-completion: behdad:~ 0$ time bash -i -c : real 0m0.571s user 0m0.397s sys 0m0.092s One should also look at what other craziness is happening in the interactive startup sequence, but bash-completion is taking more time than everything else combined. > I don't see any other effects. > Will be startup speedup notable? The speedup is notable as noted above. > Or slowdown will be much higher? A first pass would be to source the current bash-completion implementation the first time that the catch-all handler is called. That way the first tab will be slow. However, I plan to refactor bash-completion and chop it into tens or hundreds of files, such that in the catch-all handler we can only load the function needed to complete the command at hand. That's a win-win scenario, and we won't waste hundreds of kilobytes of memory per login shell either. (The bash-completion scripts are 200k. No idea how much memory bash takes to keep them in memory.) I have already created a git repo for bash-completion on fedorahosted.org. I expect to start the refactoring this weekend. Will report here. One point about the implementation. Currently I abuse the empty string to install the catch-all handler. That is: complete -F __handler '' I chose this because it minimized code change, and is already a valid but useless parameter. I personally would have preferred to use the no-parameter case as default handler. That is: complete -F __handler That's much more logical and expected. However, one can't query or remove it as "complete -p" and "complete -r" are documented as listing/removing all installed handlers. Created attachment 327033 [details]
Updated patch
"complete -p ''" was not quoting its argument correctly. Fixed that. This now can be used to detect at run time whether bash supports the catch-all completion, so I can make bash-completion work with both versions of bash with this feature and without.
Refactoring bash-completion and chopping it into tens or hundreds of files has already been done. See: http://fvue.nl/wiki/Bash_completion_library Writing a catch-all handler was the next thing on my list :-), so maybe we can join the efforts? Regards, Freddy Vulto (In reply to comment #8) > Refactoring bash-completion and chopping it into tens or hundreds of files has > already been done. See: http://fvue.nl/wiki/Bash_completion_library > > Writing a catch-all handler was the next thing on my list :-), so maybe we can > join the efforts? > > Regards, Freddy Vulto Eh, that's awesome! I'm so happy I had not started the refactoring yet. I'll study your wiki and contact you with my ideas. Cheers, behdad I just found a huge problem with this patch: it apparently changes 'complete -p command' output, by inserting spaces and carriage returns, breaking _command function. Here is a debug trace of completing on 'sudo chown <TAB>' on a host with the patch applied: ... + cmd=chown + [[ chown == -* ]] + [[ chown == -* ]] + done=1 + '[' -z 1 ']' + [[ 1 -eq 0 ]] + complete -p chown ++ complete -p chown + cspec=' complete -o filenames -F _chown chown ' + '[' '_chown chown ' '!=' ' complete -o filenames -F _chown chown ' ']' ... Here is the same trace, on a host without the patch: ... + cmd=chown + [[ chown == -* ]] + [[ chown == -* ]] + done=1 + '[' -z 1 ']' + [[ 1 -eq 0 ]] + complete -p chown ++ complete -p chown + cspec='complete -o filenames -F _chown chown' + '[' '_chown chown' '!=' 'complete -o filenames -F _chown chown' ']' This bug appears to have been reported against 'rawhide' during the Fedora 11 development cycle. Changing version to '11'. More information and reason for this action is here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping This message is a reminder that Fedora 11 is nearing its end of life. Approximately 30 (thirty) days from now Fedora will stop maintaining and issuing updates for Fedora 11. It is Fedora's policy to close all bug reports from releases that are no longer maintained. At that time this bug will be closed as WONTFIX if it remains open with a Fedora 'version' of '11'. Package Maintainer: If you wish for this bug to remain open because you plan to fix it in a currently maintained version, simply change the 'version' to a later Fedora version prior to Fedora 11's end of life. Bug Reporter: Thank you for reporting this issue and we are sorry that we may not be able to fix it before Fedora 11 is end of life. If you would still like to see this bug fixed and are able to reproduce it against a later version of Fedora please change the 'version' of this bug to the applicable version. If you are unable to change the version, please add a comment here and someone will do it for you. Although we aim to fix as many bugs as possible during every release's lifetime, sometimes those efforts are overtaken by events. Often a more recent Fedora release includes newer upstream software that fixes bugs or makes them obsolete. The process we are following is described here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping Still not fixed in rawhide, but the patch sent upstream. |