[pacman-dev] [PATCH 2/3] pacsearch with repo-agnostic coloring

[Andrew Gregory]

On 01/16/14 at 11:48am, Pierre Neidhardt wrote:
> On 14-01-16 10:13:18, Allan McRae wrote:
> > On 16/01/14 02:03, Martti Kühne wrote:
> > > On Wed, Jan 15, 2014 at 9:50 AM, Allan McRae <allan at archlinux.org> wrote:
> > > [...]
> > >>
> > >>> +     # hash function (x*2+1) is completely arbitrary.
> > >>> +     my $repohash = $v[0];
> > >>> +     $repohash =~ s/(.)/ord($1)*2+1/ge;
> > >>
> > >> I have very little perl knowledge, so I have no idea what that hash is
> > >> doing.  Can someone explain to me so I can see if that "hash" is reasonable.
> > >>
> > > 
> > > Replace each character with its [0] ascii index times two plus one?
> > > 'g' is group regexes, 'e' is eval expressions [1], as to utilize the
> > > result of the calculation.
> > > 
> > > cheers!
> > > mar77i
> > > 
> > > [0] http://perldoc.perl.org/functions/ord.html
> > > [1] http://stackoverflow.com/questions/6082219/perl-regex-e-eval-modifier-with-s
> > > 
> > 
> > Does that mean the answer can only be odd?
> 
> Ooopsy! Looks like I forgot to sum the result! Actually I didn't notice the flaw
> since it still works. The reason is that the resulting numbers are so big they
> are casted and rounded to float or sth equivalent. This is _terrible_ code
> indeed!
> 
> In the first place this was an attempt to use Perl features for a quick,
> one-line hash of strings, but since I'm not a Perl guru this may be doomed to
> fail. Any better suggestion from a Perl champion?
> 
> A more traditional way to do it:
> 
> 	sub hash_string {
> 		my $sum = 0;
> 		foreach my $l (split //, $_[0]) {
> 			$sum = $sum + 31*ord($l) + 5;
> 		}
> 		return $sum;
> 	}
> 	
>     [...]	
> 	my $repo_hash = hash_string($v[0]);
> 
> This is not a very good hash since a permutation will yield the same
> result. The following is much better
> 
> 	$sum = $sum + 31*ord($l)^$pos + 7;
> 
> but do we really need this? This is just for repo names after all, the extra
> exponentiation is superfluous in my opinion.
> 
> The offset (e.g. '+5') can be patched by other distributions make sure their
> repo have different colors.

Two suggestions:

1. Use Term::ANSIColor instead of raw ANSI color codes.  This will
   make the code more readable and make life easier for distro's that
   need to patch in their own repo colors.

2. Provide a hash variable for hard-coding repo colors.  It can be
   empty by default, making it very easy for distro's to patch in
   their official repos if the default gives poor results.

Something like:

  use Term::ANSIColor;

  my %repo_colors;

  my @colors = (
   color('blue'),
   color('red'),
   ...
  );
  ...
  my $repo_name = ...;
  my $color = ( $repo_colors{$repo_name} || $colors[ hash($repo_name) ] );

apg