Code review comment for lp:~vila/bzr/856261-unshelve-line-based

Revision history for this message
Martin Packman (gz) wrote :

The problem with the shelf_ui code is it does something totally different to all the other bazaar user interactions, hence the issues with qbzr and test scripts and editors apparently. Making it use bzrlib.ui should be the goal, but doesn't need to happen in this branch.

Benoit, that bundle is really exciting and apart from some nitpicking I think should basically be merged. Please do propose it as Vincent said.

Notes just on the changes in this branch:

+ char_based = not(os.environ.get('INSIDE_EMACS', None) is not None)
+ if char_based and not sys.stdin.isatty():

Space after 'not' otherwise it looks like a function. :)

Having modules one by one need to learn about magic emacs variables is a good argument for this logic living elsewhere.

+ # XXX: Warn if more than one char is typed ?

To get similar logic to the current and get_boolean behaviour, you'd take the first character that matched the set, which you can't do at this level without remembering the last line and only prompting for a new one when there are no more characters. That's pretty unimportant though.

« Back to merge proposal