On Wed, Sep 22, 2010 at 12:12 PM, Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > On Wed, Sep 22, 2010 at 17:59, Pat Notz <patnotz@xxxxxxxxx> wrote: >> On Tue, Sep 21, 2010 at 2:36 PM, Ævar Arnfjörð Bjarmason >> <avarab@xxxxxxxxx> wrote: >>> >>> On Tue, Sep 21, 2010 at 20:25, Pat Notz <patnotz@xxxxxxxxx> wrote: >>> >>> > +cat >editor <<\EOF >>> > +#!/bin/sh >>> > +sed -e "s/intermediate/edited/g" <"$1" >"$1-" >>> > +mv "$1-" "$1" >>> > +EOF >>> > +chmod 755 editor >>> > + >>> > +test_expect_success 'commit --squash works with -c' ' >>> > + commit_for_rebase_autosquash_setup && >>> > + EDITOR=./editor git commit --squash HEAD~1 -c HEAD && >>> > + commit_msg_is "squash! target message subject lineedited commit" >>> > +' >>> >>> Why not put the editor in t/t7500/ and use test_set_editor() like the >>> other tests? >> >> The real reason is that I'm new enough that I wasn't aware of this >> pattern. I saw what was done in t7501-commit.sh and followed along. >> I missed the use of test_set_editor() right there in t7500-commit.sh. >> Doh! >> >> I can certainly do that if it's preferred. I must say, though, that I >> find it odd to put test inputs in a separate file in a separate >> directory from where the test transforms those into expected outputs. >> To see what the test is doing you have to load both files and trace >> through it. >> >> Still, I'd be happy to change do this if that's the preferred way. > > It's a bit odd, but it's best to following existing style within a > test. Then maybe submit fixup patches to fix the whole thing later. > Yeah, there's certainly value in doing that. I'll follow-up with a v3 and include the documentation changes I noticed in the other patches. -- 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