Merge lp:~benoit.pierre/bzrtools/shell_improvements into lp:bzrtools

[Benoit Pierre]

[Benoit Pierre (benoit.pierre)]

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

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

  (fix https://bugs.launchpad.net/bzrtools/+bug/231020)

- add global config aliases to the list of possible commands when completing

- fix shell handling of alias so for example "s|grep pattern" correctly works
  if s is an alias

- add wrapper for reconfigure/switch command so prompt gets updated to reflect
  the new tree

  (fix https://bugs.launchpad.net/bzrtools/+bug/231110)

[Aaron Bentley (abentley)]

-----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? I don't see anything that specifically enables killing
commands.

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

> (fix https://bugs.launchpad.net/bzrtools/+bug/231020)

Btw, don't be shy to link your branch to the bugs it fixes. This will
show up in the merge proposal.

>
> - add global config aliases to the list of possible commands when completing
>
> - fix shell handling of alias so for example "s|grep pattern" correctly works
> if s is an alias
>
> - add wrapper for reconfigure/switch command so prompt gets updated to reflect
> the new tree
>
> (fix https://bugs.launchpad.net/bzrtools/+bug/231110)
>
>
>

> === modified file 'shell.py'
> --- shell.py 2008-02-13 04:58:32 +0000
> +++ shell.py 2009-08-02 13:35:17 +0000
> @@ -26,7 +26,7 @@
>
> from bzrlib import osutils
> from bzrlib.branch import Branch
> -from bzrlib.config import config_dir, ensure_config_dir_exists
> +from bzrlib.config import config_dir, ensure_config_dir_exists, GlobalConfig
> from bzrlib.commands import get_cmd_object, get_all_cmds, get_alias
> from bzrlib.errors import BzrError
> from bzrlib.workingtree import WorkingTree
> @@ -36,6 +36,7 @@
>
> SHELL_BLACKLIST = set(['rm', 'ls'])
> COMPLETION_BLACKLIST = set(['shell'])
> +COMPLEX_COMMAND_CHARS = ('|', '<', '>', '&', '*', '?')

I thought using a string was cute, but if we're going to use the correct
container type, we should use a set.

>
>
> class BlackListedCommand(BzrError):
> @@ -167,18 +168,37 @@
> def do_help(self, line):
> self.default("help "+line)
>
> + def exec_and_update_tree(self, line):
> + result = self.default(line)
> + try:
> + self.tree = WorkingTree.open_containing(".")[0]
> + except:
> + self.tree = None
> + return result
> +
> + def do_switch(self, line):
> + return self.exec_and_update_tree('switch ' + line)

This is okay, but I think it would be better to just put 'switch' and
'reconfigure' in a set like the COMPLETION_BLACKLIST, and then update
the tree when commands in that set are encountered.

> + def do_reconfigure(self, line):
> + return self.exec_and_update_tree('reconfigure ' + line)
> +
> def def...

review: Needs Resubmitting

[Benoit Pierre (benoit.pierre)]

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

Unmerged revisions

692. By Benoit Pierre on 2009-08-02

Catch shlex.split ValueError exceptions.

691. By Benoit Pierre on 2009-08-02

Merge with upstream.

690. By Benoit Pierre on 2008-11-30

Add global config aliases to the list of possible commands when
completing in the shell.

689. By Benoit Pierre on 2008-11-30

Fix shell handling of alias so for example "s|grep pattern" correctly
works if s is an alias.

688. By Benoit Pierre on 2008-11-30

Factorize code in shell to use the same set of chars to detect complex
command line.

687. By Benoit Pierre on 2008-11-30

In shell, always update the prompt after a switch/reconfigure, even if
the result of the command is non 0 because in case of a complex command
we may actually not be testing the result of the switch/reconfigure
command itself, like for example: switch ../trunk | false.

686. By Benoit Pierre on 2008-11-30

Merge with upstream.

685. By Benoit Pierre on 2008-11-23

Factorize code and add wrapper for reconfigure command so prompt gets
updated to reflect the new tree.

684. By Benoit Pierre on 2008-11-19

Add shell switch builtin which is used to wrap the call to the bzr switch
command so we can update the shell prompt accordingly.

683. By Benoit Pierre on 2008-11-19

Allows killing the current command line and start with a new one with ^C
like most shells do (which also helps avoiding to kill the shell by
mistake when one's timing is off when trying to interrupt a command in
progress).

=== modified file 'shell.py'
--- shell.py	2008-02-13 04:58:32 +0000
+++ shell.py	2009-08-02 13:35:17 +0000
@@ -26,7 +26,7 @@
 
 from bzrlib import osutils
 from bzrlib.branch import Branch
-from bzrlib.config import config_dir, ensure_config_dir_exists
+from bzrlib.config import config_dir, ensure_config_dir_exists, GlobalConfig
 from bzrlib.commands import get_cmd_object, get_all_cmds, get_alias
 from bzrlib.errors import BzrError
 from bzrlib.workingtree import WorkingTree
@@ -36,6 +36,7 @@
 
 SHELL_BLACKLIST = set(['rm', 'ls'])
 COMPLETION_BLACKLIST = set(['shell'])
+COMPLEX_COMMAND_CHARS = ('|', '<', '>', '&', '*', '?')
 
 
 class BlackListedCommand(BzrError):
@@ -167,18 +168,37 @@
     def do_help(self, line):
         self.default("help "+line)
 
+    def exec_and_update_tree(self, line):
+        result = self.default(line)
+        try:
+            self.tree = WorkingTree.open_containing(".")[0]
+        except:
+            self.tree = None
+        return result
+
+    def do_switch(self, line):
+        return self.exec_and_update_tree('switch ' + line)
+
+    def do_reconfigure(self, line):
+        return self.exec_and_update_tree('reconfigure ' + line)
+
     def default(self, line):
-        args = shlex.split(line)
-        alias_args = get_alias(args[0])
-        if alias_args is not None:
-            args[0] = alias_args.pop(0)
+        try:
+            args = shlex.split(line)
+        except ValueError, e:
+            print 'ValueError:', e
+            return
+        alias_args = None
 
         commandname = args.pop(0)
-        for char in ('|', '<', '>'):
+        for char in COMPLEX_COMMAND_CHARS:
             commandname = commandname.split(char)[0]
-        if commandname[-1] in ('|', '<', '>'):
+        if commandname[-1] in COMPLEX_COMMAND_CHARS:
             commandname = commandname[:-1]
         try:
+            alias_args = get_alias(commandname)
+            if alias_args is not None:
+                commandname = alias_args.pop(0)
             if commandname in SHELL_BLACKLIST:
                 raise BlackListedCommand(commandname)
             cmd_obj = get_cmd_object(commandname)
@@ -223,10 +243,13 @@
 def run_shell():
     try:
         prompt = PromptCmd()
-        try:
-            prompt.cmdloop()
-        finally:
-            prompt.write_history()
+        while True:
+            try:
+                prompt.cmdloop()
+            except KeyboardInterrupt:
+                print
+            finally:
+                prompt.write_history()
     except StopIteration:
         pass
 
@@ -280,6 +303,8 @@
 
 
 def iter_command_names(hidden=False):
+    for alias in GlobalConfig().get_aliases().keys():
+        yield alias
     for real_cmd_name, cmd_class in get_all_cmds():
         if not hidden and cmd_class.hidden:
             continue
@@ -312,7 +337,7 @@
 
 
 def too_complicated(line):
-    for char in '|<>*?':
+    for char in COMPLEX_COMMAND_CHARS:
         if char in line:
             return True
     return False