On Mon, Feb 04, 2013 at 02:54:30PM -0500, Ted Zlatanov wrote:

> +     print <<EOHIPPUS;
> +
> +$0 [-f AUTHFILE] [-d] get
> +
> +Version $VERSION by tzz\@lifelogs.com.  License: BSD.

This here-doc is interpolated so you can use $0 and $VERSION, and
therefore have to quote the @-sign. But later in the here-doc...

> +Thus, when we get "protocol=https\nusername=tzz", this credential
> +helper will look for lines in AUTHFILE that match

Do you need to quote "\n" here?

> +die "Sorry, you need to specify an existing netrc file (with or without a 
> .gpg extension) with -f AUTHFILE"
> + unless defined $file;
> +
> +unless (-f $file) {
> +     print STDERR "Sorry, the specified netrc $file is not accessible\n" if 
> $debug;
> +     exit 0;
> +}

Hmm, so it's not an error (just a warning) to say:

  git credential-netrc -f /does/not/exist

but it is an error to say:

  git credential-netrc

and have it fail to find any netrc files. Shouldn't the latter be a
lesser error than the former?

> +while (<STDIN>) {
> +     next unless m/([^=]+)=(.+)/;
> +
> +     my ($token, $value) = ($1, $2);
> +     die "Unknown search token $1" unless exists $q{$token};
> +     $q{$token} = $value;
> +}

Should this regex be anchored at the start of the string? I think the
left-to-right matching means we will correctly match:

  key=value with=in it

so it may be OK.

> +if ($debug) {
> +     printf STDERR "searching for %s = %s\n", $_, $q{$_} || '(any value)'
> +      foreach sort keys %q;
> +}

Leftover one-char indent.

> [...]

The rest looks OK to me.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to