On Mon, Feb 11, 2013 at 05:23:38PM +0100, Michal Nazarewicz wrote: > +=item credential_read( FILE_HANDLE ) > + > +Reads credential key-value pairs from C<FILE_HANDLE>. Reading stops at EOF or > +when an empty line is encountered. Each line must be of the form C<key=value> > +with a non-empty key. Function returns a hash with all read values. Any > +white space (other then new-line character) is preserved. > + > +=cut > + > +sub credential_read { > + my ($self, $reader) = _maybe_self(@_); > + my %credential; > + while (<$reader>) { > + chomp; > + if ($_ eq '') { > + last; > + } elsif (!/^([^=]+)=(.*)$/) { > + throw Error::Simple("unable to parse git credential data:\n$_"); > + } > + $credential{$1} = $2; > + } > + return %credential; > +} Should this return a hash reference? It seems like that is how we end up using and passing it elsewhere (since we have to anyway when passing it as a parameter). I don't have a strong preference, and it's somewhat a matter of taste. And maybe returning the actual hash matches the rest of the module better. I admit I don't really use Git.pm much. -Peff -- 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