Hi,
On Tue, 27 Jan 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>
> > Rather than copying and pasting, which is prone to lead to fixes
> > missing in one version, move the fake-editor generator to t/t3404/.
> >
> > While at it, fix a typo that causes head-scratching: use
> > ${SHELL_PATH-/bin/sh} instead of $SHELL_PATH.
>
> I've learned to be cautious whenever I see "while at it".
Heh.
> > diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> > new file mode 100644
> > index 0000000..8c8caab
> > --- /dev/null
> > +++ b/t/lib-rebase.sh
> > @@ -0,0 +1,36 @@
> > +#!/bin/sh
> > +
> > +set_fake_editor () {
> > + echo "#!${SHELL_PATH-/bin_sh}" >fake-editor.sh
>
> It is unclear why you would want to do this. It was unclear what "typo"
> you were referring to in your commit log message, either.
>
> The tests are supposed to run under the shell the user specified, so if
> there is a case you found that $SHELL_PATH is unset, that is a bug we
> would want to fix, and ${SHELL_PATH-/bin/sh} is sweeping the problem under
> the rug to make it harder to fix, isn't it?
I call the scripts directly, and I do not think it would be a good idea to
force the user to use GIT_TEST_OPTS and make when calling the script
directly is so much easier. Plus, this way I can pass "sh -x $SCRIPT"
easily.
I am really puzzled that it works, BTW. With an empty SHELL_PATH,
apparently.
> Besides, it's /bin/sh, not /bin_sh ;-)
Right. The commit message was right, at least!
> > + cat >> fake-editor.sh <<\EOF
> > +case "$1" in
> > +*/COMMIT_EDITMSG)
> > + test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1"
> > + test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1"
> > + exit
> > + ;;
> > +esac
> > +test -z "$EXPECT_COUNT" ||
> > + test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) ||
> > + exit
> > +test -z "$FAKE_LINES" && exit
> > +grep -v '^#' < "$1" > "$1".tmp
> > +rm -f "$1"
> > +cat "$1".tmp
> > +action=pick
> > +for line in $FAKE_LINES; do
> > + case $line in
> > + squash|edit)
> > + action="$line";;
> > + *)
> > + echo sed -n "${line}s/^pick/$action/p"
> > + sed -n "${line}p" < "$1".tmp
> > + sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
>
> I looked at the output from this and wondered what these "sed -n" shown
> in the "-v" output were about last night. I do think it is a good idea
> to show what edit was done to the insn stream, but I suspect it may be
> easier to read the output if you did this instead:
>
> > + sed -n "${line}p" < "$1".tmp
> > + sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
> > + sed -n "${line}s/^pick/$action/p" < "$1".tmp
Probably. It is for debugging, anyway. As everything you only see with
-v.
Ciao,
Dscho
--
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
