Merge lp://qastaging/~gz/bzrtools/shell_without_readline_719075 into lp://qastaging/bzrtools
Proposed by
Martin Packman
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 764 | ||||
Proposed branch: | lp://qastaging/~gz/bzrtools/shell_without_readline_719075 | ||||
Merge into: | lp://qastaging/bzrtools | ||||
Diff against target: |
39 lines (+13/-6) 1 file modified
shell.py (+13/-6) |
||||
To merge this branch: | bzr merge lp://qastaging/~gz/bzrtools/shell_without_readline_719075 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Aaron Bentley | Needs Fixing | ||
Review via email: mp+49744@code.qastaging.launchpad.net |
Description of the change
Allows the readline import to fail in bzrtools.shell and modifies PromptCmd to tolerate the absence of the readline module. The autocompletion is explictly disabled if this is the case, it would work anyway, but it saves cmd.Cmd repeatedly trying and failing to import readline.
The test is a little black magic, but I think it should be reliable, and function on all platforms to ensure this doesn't accidentally regress.
To post a comment you must log in.
Making readline optional is fine, but please retain PEP8 import ordering-- readline should still be imported between os and shlex, not as if it was a third-party library.
I don't understand why you're explicitly disabling autocompletion. On platforms without readline, it won't work, but I don't think that incurs overhead.
Your test is broken. It passes (on Ubuntu Maverick) even if I revert the changes to shell.py. I believe it is sensitive to differences in python path.
I don't think it's sensible to test interactive functionality using a blackbox test. Tests are not required for "shell". Actually, all of bzrtools is a bit lax.