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

Revision history for this message
Guilherme Salgado (salgado) wrote :

On Wed, 2010-01-06 at 21:03 +0000, John A Meinel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Guilherme Salgado wrote:
> > On Wed, 2010-01-06 at 18:30 +0000, John A Meinel wrote:
> >> -----BEGIN PGP SIGNED MESSAGE-----
> >> Hash: SHA1
> >>
> >>
> >> ...
> >>
> >>> I could do that, but I'd prefer to just let the changes get written to
> >>> the terminal if you don't see that as a problem.
> >> Having discussed it a bit with Aaron, it seems that shelf_ui.Unshelver
> >> is meant to be only used for command-line operation, while
> >> shelf.Unshelver is the class meant to be used for guis, etc. (The actual
> >> functionality is supposed to be in the latter, managing it from a CLI is
> >> meant to be in the former.)
> >>
> >> As such, we wouldn't have to pass the output file after all, as it is
> >> okay for Unshelver to determine this itself.
> >>
> >> That said, it is certainly easier to test it, if we allow it to be
> >> passed in. :)
> >
> > Given that it'll probably be used only for testing, I think it'd make
> > sense to have only run() and write_diff() accept the optional file-like
> > object instead of having it in from_args(), __init__() and as an
> > instance variable. Do you agree?
> >
>
> At this point, I don't care either way. Do what feels good to you :).

Ok, I'll leave it as is, then.

>
>
> The main issue is that regexes don't really like to match across lines.
> A different possibility would be:
>
> expected = """\
> @@ -1,4 +1,4 @@
> - -a
> +z
> b
> c
> d
> @@ -7,4 +7,4 @@
> g
> h
> i
> - -j
> +y
> """
>
> self.assertEqualDiff(expected, content[-len(expected):])
>
> eg, match the last bytes of content and make sure they match what you
> expect.

That works; I just used dedent() so that I could have the multi-line
string properly indented.

Shall I re-submit this m-p?

--
Guilherme Salgado <email address hidden>

« Back to merge proposal