Code review comment for lp:~salgado/bzr/bug-308122

I think the helper would be nice, but not critical. I've wanted it from time to time as well.

I think that displaying the diff vs just the overview should probably be a flag on Unshelver, though it may just always get set by the same command-line flag. Something like:

show_changes(self, merger, include_diff=False/True).

This may be a YAGNI, but it seems like better layering. Arguably if we show the diff, we *shouldn't* show the summary.

Actually, I think my bigger concern is writing directly to stdout. Consider that something like "bzr qshelve" might really like this functionality, I think we should pass in the file to write to.

And if you did that, you wouldn't need the "replace_stdout()" helper.

Looking at cmd_unshelve, though I see why this is slightly harder. cmd_unshelve defers everything to 'Unshelver.run()' which doesn't take anything like an output file.

I think it should, though. Another possibility is that we should at least use "ui.ui_factory.make_output_stream()". Since that will mean that progress bars won't interfere with the diff output.

I'm a little concerned about the UI impact. In that this changes 'unshelve --dry-run' to *always* show the diff, when it did not used to. (--dry-run --verbose?). I personally think always showing the diff is probably fine, as I don't really see the short form being very informative. (Consider if we changed 'bzr st' to suddenly be equivalent to 'bzr diff')