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

> 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.

/me nods

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

/me .. err, sorry :)

>
> 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. :)

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 ?

>
> 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.

And would keep the same UI which can drive any user nuts, it's really
intended to be line-based with one asnwer by *line* ending with CR, no
buffering of answers there.

What made shelve unsuable for me is that I needed to prepare the answers for
all the questions *before* seeing the questions, keeping this logic just
don't work ;)

So my question may be clearer as:

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

Also, I need to add tests and that will probably remove the weird

  char = '\n'

just-in-case hack.