On Sun, Nov 14, 2010 at 18:32, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > Ãvar ArnfjÃrà Bjarmason wrote: > >>              I used ! instead of test_must_fail so that >> the GIT_EDITOR variable was only used in this command invocation, > >    Â( >        ÂGIT_EDITOR=cat && >        Âexport GIT_EDITOR && >        Âtest_must_fail ... >    Â) > > ? > >>                                  and >> because the surrounding tests use this style. > > Oh, okay. ÂI had the test_must_fail version sitting in my local tree > as part of the unsent part of the &&-chaining series > http://thread.gmane.org/gmane.comp.version-control.git/157903/focus=160419 > > I am happy to see this fixed sooner. Better to fix it up later IMO in something like your series. One surgery at a time. > [...] >> +test_expect_success \ >> +   'message in editor has initial comment: first line' ' >>    # check the first line --- should be empty >> -   first=$(sed -e 1q <actual) && >> -   test -z "$first" && >> +   echo >first.expect && >> +   sed -e 1q <actual >first.actual && >> +   test_cmp first.expect first.actual >> +' >> + >> +test_expect_success \ >> +   'message in editor has initial comment: remainder' ' >>    # remove commented lines from the remainder -- should be empty >> -   rest=$(sed -e 1d -e '/^#/d' <actual) && >> -   test -z "$rest" >> +   >rest.expect >> +   sed -e 1d -e '/^#/d' <actual >rest.actual && >> +   test_cmp rest.expect rest.actual >> Â' > > Nice. ÂNote that you've changed the semantics of the test here > (probably for the better): the $() backquoting operator strips > trailing newlines, while your sed expression does not. > > I assume you've run the tests and made sure they still pass. Yeah, I've been running all these (also the others in the series) for months on several different platforms with different options. They all work. > For what it's worth, > Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx> Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html