Junio C Hamano wrote:
> Jakub Narebski <jnareb@xxxxxxxxx> writes:
>
> * gitweb: Restore old git_blame using git-annotate under "annotate"
>
> I actually was hoping to see a patch to remove the git_blame
> which is not used as far as I can see.
>
> Although we carried annotate and blame in git.git tree for
> quite a while, the intention was always to deprecate one over
> the other once pros-and-cons of each implementation become
> clear (and I think people would not miss annotate if we remove
> annotate and make it an alias for blame -c anymore). What's
> the reason we would want to have both?
Well, removing it completely might be not a best idea for now,
as git_annotate has for example two more columns ("Age" and "Author")
which might be usefull.
I guess the intention is to provide patch for those who want
to improve git_blame, to have comparison with git_annotate,
i.e. patch not applied but available.
> * gitweb: Use 'local $/ = undef;' before 'print <$fd>;'
>
> You changed:
>
> $/ = undef;
> print <$fd>;
> ... hope that nobody depends on standard value of $/
> ... around here, which may still break if you did sub
> ... calls, the sub did not localize $/ (who would?),
> ... and depended to have a sane $/.
> $/ = "\n";
>
> to
>
> local $/ = undef;
> print <$fd>;
> ... hope that nobody depends on standard value of $/
> ... until the end of scope, and whoever changes this
> ... sub is careful enough in the future
>
> which I think is worse. Introducing an extra scope explicitly
> delimit the part you want to use localized $/ like this
>
> { local $/; print <$fd>; }
>
> might have been more palatable. Am I guessing the reason of
> your change wrong?
Actually only one change was from '$/ = undef; print <$fd>; $/ = "\n"',
namely at the end of git_blob_plain. I think nobody will print anything
to the end of the sub.
Two other chunks actually _introduced_ 'local $/ = undef', both in the
fairly short 'if' body. First to read $home_text (there change is not that
important, as $home_text should be fairly short), next more important in
git_snapshot.
IMVHO 'local $/ = undef' in all commands that get the rest of the output
from filehandle (and outputting nothing later), like all 'plain' outputs
should be the idiom to use.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
-
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