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

Revision history for this message
Martin Packman (gz) wrote :

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

Ha, the problem with writing up some comments one evening and posting them the next, you're way too fast. :)

The new unicode test looks like a good start, and sorry, should have mentioned failures on tests like that tend to break things badly currently, you'll want testtools 0.9.5 or later and either <lp:~gz/bzr/escape_selftest_console_output_633216> or a utf-8 console. However, there does need to be a test that actually launches some kind of real subprocess, as that's where non-ascii filenames are most likely to break down. Don't need a real merge tool, echo on the shell would probably do.

I agree storing lists in the config file hurts editability. Slightly annoying as to get back to a string does mean quoting even though templated command lines are unlikely to ever need it. On windows you can use subprocess.list2cmdline but you might want your quoting function back for nix even though `" ".join(map(repr, commandline))` should basically work.

« Back to merge proposal