Code review comment for lp://qastaging/~benoit.pierre/bzrtools/shell_improvements

Revision history for this message
Benoit Pierre (benoit.pierre) wrote :

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi Benoit,
>
> Thanks for your patch. I'm sorry about the delay responding.
>
> In general, please submit changes separately. It wasn't easy for me to
> review initially, which contributed to the delay.
>
> review resubmit
>
> Benoit PIERRE wrote:
> > Benoit PIERRE has proposed merging
> lp:~benoit.pierre/bzrtools/shell_improvements into lp:bzrtools.
> >
> > Requested reviews:
> > Aaron Bentley (abentley)
> >
> > Various improvements for the shell command:
> >
> > - allow killing the current command line and start with a new one with ^C like
> > most shells do (which also helps avoiding killing the shell by mistake when
> > one's timing is off when trying to interrupt a command in progress), ^D can
> > be used to exit when on an empty line
>
> I think what you mean is that ^C no longer kills the shell permanently.
> Is that right?

Yes, separate merge proposal: https://code.launchpad.net/~benoit.pierre/bzrtools/shell_improvement_kbd_interrupt/+merge/12187

>
> > - catch shlex.split ValueError exceptions: the erroneous command can be edited
> > again by using <up> to navigate the readline history
>
> What can cause a ValueError?

From the shlex code: missing closing '/" or EOF after a \.

Separate merge proposal: https://code.launchpad.net/~benoit.pierre/bzrtools/shell_improvement_parse_errors/+merge/12188

Separate merge proposal for the rest will follow when I have time.

« Back to merge proposal