BzrTools

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

  1. shell_improvements
  2. Merge into bzrtools.dev
Proposed by Benoit Pierre
Status: Rejected
Rejected by: Aaron Bentley
Proposed branch: lp://qastaging/~benoit.pierre/bzrtools/shell_improvements
Merge into: lp://qastaging/bzrtools
Diff against target: None lines
To merge this branch: bzr merge lp://qastaging/~benoit.pierre/bzrtools/shell_improvements
Reviewer Review Type Date Requested Status
Aaron Bentley Needs Resubmitting
Review via email: mp+9547@code.qastaging.launchpad.net
To post a comment you must log in.
Revision history for this message
Benoit Pierre (benoit.pierre) wrote :

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)

Revision history for this message
Aaron Bentley (abentley) wrote :
Download full text (5.5 KiB)

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

Read more...

review: Needs Resubmitting
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.

Unmerged revisions

692. By Benoit Pierre

Catch shlex.split ValueError exceptions.

691. By Benoit Pierre

Merge with upstream.

690. By Benoit Pierre

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

689. By Benoit Pierre

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

688. By Benoit Pierre

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

687. By Benoit Pierre

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

Merge with upstream.

685. By Benoit Pierre

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

684. By Benoit Pierre

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

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
2626
27from bzrlib import osutils27from bzrlib import osutils
28from bzrlib.branch import Branch28from bzrlib.branch import Branch
29from bzrlib.config import config_dir, ensure_config_dir_exists29from bzrlib.config import config_dir, ensure_config_dir_exists, GlobalConfig
30from bzrlib.commands import get_cmd_object, get_all_cmds, get_alias30from bzrlib.commands import get_cmd_object, get_all_cmds, get_alias
31from bzrlib.errors import BzrError31from bzrlib.errors import BzrError
32from bzrlib.workingtree import WorkingTree32from bzrlib.workingtree import WorkingTree
@@ -36,6 +36,7 @@
3636
37SHELL_BLACKLIST = set(['rm', 'ls'])37SHELL_BLACKLIST = set(['rm', 'ls'])
38COMPLETION_BLACKLIST = set(['shell'])38COMPLETION_BLACKLIST = set(['shell'])
39COMPLEX_COMMAND_CHARS = ('|', '<', '>', '&', '*', '?')
3940
4041
41class BlackListedCommand(BzrError):42class BlackListedCommand(BzrError):
@@ -167,18 +168,37 @@
167 def do_help(self, line):168 def do_help(self, line):
168 self.default("help "+line)169 self.default("help "+line)
169170
171 def exec_and_update_tree(self, line):
172 result = self.default(line)
173 try:
174 self.tree = WorkingTree.open_containing(".")[0]
175 except:
176 self.tree = None
177 return result
178
179 def do_switch(self, line):
180 return self.exec_and_update_tree('switch ' + line)
181
182 def do_reconfigure(self, line):
183 return self.exec_and_update_tree('reconfigure ' + line)
184
170 def default(self, line):185 def default(self, line):
171 args = shlex.split(line)186 try:
172 alias_args = get_alias(args[0])187 args = shlex.split(line)
173 if alias_args is not None:188 except ValueError, e:
174 args[0] = alias_args.pop(0)189 print 'ValueError:', e
190 return
191 alias_args = None
175192
176 commandname = args.pop(0)193 commandname = args.pop(0)
177 for char in ('|', '<', '>'):194 for char in COMPLEX_COMMAND_CHARS:
178 commandname = commandname.split(char)[0]195 commandname = commandname.split(char)[0]
179 if commandname[-1] in ('|', '<', '>'):196 if commandname[-1] in COMPLEX_COMMAND_CHARS:
180 commandname = commandname[:-1]197 commandname = commandname[:-1]
181 try:198 try:
199 alias_args = get_alias(commandname)
200 if alias_args is not None:
201 commandname = alias_args.pop(0)
182 if commandname in SHELL_BLACKLIST:202 if commandname in SHELL_BLACKLIST:
183 raise BlackListedCommand(commandname)203 raise BlackListedCommand(commandname)
184 cmd_obj = get_cmd_object(commandname)204 cmd_obj = get_cmd_object(commandname)
@@ -223,10 +243,13 @@
223def run_shell():243def run_shell():
224 try:244 try:
225 prompt = PromptCmd()245 prompt = PromptCmd()
226 try:246 while True:
227 prompt.cmdloop()247 try:
228 finally:248 prompt.cmdloop()
229 prompt.write_history()249 except KeyboardInterrupt:
250 print
251 finally:
252 prompt.write_history()
230 except StopIteration:253 except StopIteration:
231 pass254 pass
232255
@@ -280,6 +303,8 @@
280303
281304
282def iter_command_names(hidden=False):305def iter_command_names(hidden=False):
306 for alias in GlobalConfig().get_aliases().keys():
307 yield alias
283 for real_cmd_name, cmd_class in get_all_cmds():308 for real_cmd_name, cmd_class in get_all_cmds():
284 if not hidden and cmd_class.hidden:309 if not hidden and cmd_class.hidden:
285 continue310 continue
@@ -312,7 +337,7 @@
312337
313338
314def too_complicated(line):339def too_complicated(line):
315 for char in '|<>*?':340 for char in COMPLEX_COMMAND_CHARS:
316 if char in line:341 if char in line:
317 return True342 return True
318 return False343 return False

Subscribers

People subscribed via source and target branches