On Jan 12, 2008 4:32 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Ping Yin <pkufranky@xxxxxxxxx> writes:
>
>
> > + check_dst=t
> > + check_src=t
> > + case $status in
> > + D)
> > + check_dst=
> > + ;;
> > + A)
> > + check_src=
> > + ;;
>
> I'd loosen the above grep (see my comments to your 1/5) and also
> add this:
>
> *)
> continue ;# punt
> ;;
>
> so that the rest of the code won't break when seeing a path that
> was submodule in the HEAD but is a blob in the index.
Right. I thought 'M' should be the default case because I missed the
case 'T' (head blob but index submodule, or the reverse) .
>
> > + esac
> > +
> > + (
> > + errmsg=
> > + unfound_src=
> > + unfound_dst=
> > +
> > + test -z "$check_src" ||
> > + GIT_DIR="$name/.git" git-rev-parse $sha1_src >&/dev/null ||
>
>
> I think you would want to read full 40-char sha1_src and
> sha1_dst with "while read", and keep that full 40-char in these
> variables, and use them when calling rev-parse here.
Hmm, precision is really a problem. However, "git diff --raw" will not
always give full 40-char sha1, instead it will give sha1 with enough
length. So maybe i can use the sha1 from "git diff --raw" ?
>
> If you are checking if that the object exists in the submodule,
> use "rev-parse --verify", which was designed for exactly that
> purpose. If you also want to verify if the object is a commit,
> which may be a good idea anyway, "rev-parse --verify $sha1_src^0".
Yes, --verify is better.
;;
>
> When reporting errors, you would want to give full 40-chars...
>
As said before, enough is ok?
> > + *)
> > + left=
> > + right=
> > + test -n "$check_src" &&
> > + left=$(GIT_DIR="$name/.git" git log --pretty=format:" <%s" \
> > + ${check_dst:+$sha1_dst..}$sha1_src 2>/dev/null)
> > +
> > + test -n "$check_dst" &&
> > + right=$(GIT_DIR="$name/.git" git log --reverse --pretty=format:" >%s" \
> > + ${check_src:+$sha1_src..}$sha1_dst 2>/dev/null)
> > + ;;
> > + esac
> > +
> > + echo "* $name $sha1_src...$sha1_dst:"
>
> While reporting like this, you would want the shortened form,
> perhaps produced your "cut -c1-7".
>
> > + if test -n "$errmsg"
> > + then
> > + echo "$errmsg"
> > + else
> > + test -n "$left" && echo "$left"
> > + test -n "$right" && echo "$right"
> > + fi
> > + echo
> > + ) | sed 's/^/# /'
> > + done
>
> I'd prefer to always have "-e" before the sed expression.
Is it not portable without "-e"?
>
> Any reason why you want separate invocation of sed inside the
> while loop? IOW, why isn't it like this?
>
> git diff --raw |
> while read ...
> do
> ...
> done | sed -e 's/^/# /'
>
Just because i'm stupid. :)
--
Ping Yin
-
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
