Code review comment for lp:~simon-kersey/bzr-explorer/add-tool-cmd-selected-support

Revision history for this message
Simon Kersey (simon-kersey) wrote :

> Simon,
>
> I don't understand why do you put single quotes around multiple paths as in
> code:
>
> + self.location_context['wt_selected'] = "'"+ \
> + ("' '".join(paths))+"'"

The reason they are quoted is to cope with paths that contain spaces.

The reason they are quoted with ' rather than " is that "s get removed when populating the qrun options and arguments list.

I'm not sure whether it is the call to self._escape_context or mod_app_suite.command_to_args (line 992) or the call to self.run_app (line 1004) or indeed qrun itself that does it. Without the quotes, any path with spaces is parsed as separate arguments by qrun giving the wrong result.

If you add the following tools to tools.xml (Tools->Edit My Tools) then you can test the behavior with multiple file selections:
  <tool action="add --dry-run %(wt_selected)s" icon="actions/document-properties" title="Add" type="bzr" />
  <tool action="add --dry-run %(wt_selected)s" icon="actions/edit-select-all" title="Add Now" type="bzr-exec" />

(The example tools I listed in the testing section above are based on bzr ls which only takes a single path as an argument and so it will give an error if multiple paths are selected.)

« Back to merge proposal