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

So, to be clear, I think this branch may as well land as an improvement to the current isatty() hack, with the thought that Benoit's change will be the basis for a longer term solution.

> > + char_based = not(os.environ.get('INSIDE_EMACS', None) is not None)

...or just `is None` for that matter.

> Yeah, thanks. But the plan is not to mention emacs at all there but instead
> let user configure that themselves (INSIDE_EMACS is a far too big hammer and
> was just a quick way to test but it's not reliable nor appropriate for all
> cases), any feedback on the proposed config variables ?

For now it seems an appropriate hammer, just hang it elsewhere in a little private function. After this is all moved to bzrlib.ui then a config value to enable/disable character based input seems reasonable, but I wouldn't bother for now.

> - given that we expect only a single char (followed by a \n), what should we
> do if we get more ? Warn ? Swallow silently ? Fail ?

Swallow (and maybe warn) or fail and reprompt would all be fine.

> Also, I need to add tests and that will probably remove the weird
>
> char = '\n'
>
> just-in-case hack.

Right, you probably want a test for the EOF case, it needs to make sure it gets out of the shelf loop somehow.