Fredrik Gustafsson <iveqy@xxxxxxxxx> writes:
> On Thu, May 31, 2012 at 11:19:04AM +0200, Johannes Sixt wrote:
>> Am 5/31/2012 10:48, schrieb Fredrik Gustafsson:
>> > Rewrote a perl section in sh.
>>
>> > The code may be a bit slower (doing grep on strings instead of using
>> > perl-lists).
>>
>> "A lot" would be more correct on Windows :-) But it can be avoided, I think.
>>
>> > module_list()
>> > {
>> > + unmerged=
>> > + null_sha1=0000000000000000000000000000000000000000
>> > git ls-files --error-unmatch --stage -- "$@" |
>> > - perl -e '
>> > - my %unmerged = ();
>> > - my ($null_sha1) = ("0" x 40);
>> > - while (<STDIN>) {
>> > - chomp;
>> > - my ($mode, $sha1, $stage, $path) =
>> > - /^([0-7]+) ([0-9a-f]{40}) ([0-3])\t(.*)$/;
>> > - next unless $mode eq "160000";
>> > - if ($stage ne "0") {
>> > - if (!$unmerged{$path}++) {
>> > - print "$mode $null_sha1 U\t$path\n";
>> > - }
>> > - next;
>> > - }
>> > - print "$_\n";
>> > - }
>> > - '
>> > + while read mode sha1 stage path
>>
>> Be prepared for backslashes in the path name:
>>
>> while read -r mode sha1 stage path
>
> We are not using -r on any place in git-submodule.sh. Maybe we should? I
> can provide a patch if needed.
I've already written off "git-submodule.sh" script as unfriendly to
pathnames with funny characters in them. Using "read -r" may work
around backslashes in the path, but you won't be able to sensibly
handle LFs in filenames, for example. In other words, we could just
tell users "don't use funny pathnames"---and from that point of
view, rewriting the Perl scriptlet with a pure shell implementation
that does not fork other little tools is sensible, especially if it
results in better performance, more readable code, etc.
Having said that, in the longer term, I think the right direction to
go is the opposite. It would be better to make "git-submodule.sh"
work better with paths with funny characters in them, and one
obvious approach is to read "ls-files -z" output with something
capable of parsing NUL-terminated records, e.g. a Perl scriptlet.
Adding a new shell loop like this patch only adds one place that
needs to be fixed later when that happens, so I am not sure I like
this patch.
--
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