From: Junio C Hamano <junkio@cox.net>
Date: Thu, 17 Aug 2006 09:55:51 GMT
Subject: Re: [PATCH 5/7] gitweb: Added parse_difftree_raw_line function for later use
Message-ID: <7vfyfvsn2w.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <11558064893592-git-send-email-jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> +sub parse_difftree_raw_line {
> +	my $line = shift;
> +	my %res;
> +
> +	# ':100644 100644 03b218260e99b78c6df0ed378e59ed9205ccc96d 3b93d5e7cc7f7dd4ebed13a5cc1a4ad976fc94d8 M	ls-files.c'
> +	# ':100644 100644 7f9281985086971d3877aca27704f2aaf9c448ce bc190ebc71bbd923f2b728e505408f5e54bd073a M	rev-tree.c'
> +	if ($line =~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)([0-9]{0,3})\t(.*)$/) {

We would probably want to say that status symbol is an uppercase
alpha [A-Z].  Or maybe limit it to a non-whitespace letter (\S).
Dot there to say "just any letter" looks wrong.

I would use "+" instead of hardcoding the length of mode strings
(you have 6 there) and object name hash (ditto 40) but that is
mostly style and a slight future proofing against us using
longer hashes (not in our immediate future though).