Code review comment for lp:~doxxx/bzr/mergetools

> I see _optional_quote_arg is still being used. I think you should be storing a
> list not a string in self._commandline so you can delete that and the related
> functions.

I pushed new revisions almost a day ago which did exactly that.

> We don't want the user to be doing any quoting. Needing to wrap every %var in
> quotes just in case it contains a space is shell-level stupidity we're
> avoiding by handling that for them.

I meant quoting which may be necessary for passing arguments correctly to the merge tool they're using. I agree that the user should not have to quote the substitution markers in case they're used with files containing spaces. That's part of the problem with the current system.

> I really don't like grovelling through the filesystem with _find_executable as
> an idea. (Does it even have a clear licence?) The 'detect' command for merge
> tools does seem reasonable though so maybe it's unavoidable.

That find_executable function is exactly as it appears on the webpage I reference in the comment above it. There is no license that I can see. Since it was on a site that explicitly accepts and re-publishes "snippets" of code, I assumed that it was basically in the public domain.

I really like the detect command because it takes a lot of the work out of configuring the merge tools, which is one of the primary goals I'm trying to accomplish here.