Re: [PATCH 5/6] Git.pm: teach "ident" to query explicitness

2012-11-14 Thread Jeff King
On Wed, Nov 14, 2012 at 09:12:13AM -0800, Jonathan Nieder wrote:

> > --- a/perl/Git.pm
> > +++ b/perl/Git.pm
> > @@ -737,7 +737,7 @@ sub remote_refs {
> >  }
> >  
> >  
> > -=item ident ( TYPE | IDENTSTR )
> > +=item ident ( TYPE | IDENTSTR [, options] )
> >  
> >  =item ident_person ( TYPE | IDENTSTR | IDENTARRAY )
> >  
> > @@ -750,6 +750,10 @@ and either returns it as a scalar string or as an 
> > array with the fields parsed.
> >  Alternatively, it can take a prepared ident string (e.g. from the commit
> >  object) and just parse it.
> >  
> > +If the C option is set to 1, the returned array will contain an
> > +additional boolean specifying whether the ident was configure explicitly 
> > by the
> > +user.
> 
> s/configure/configured/
> 
> I'd suggest adding "See GIT_COMMITTER_EXPLICIT in git-var(1) for
> details" to make the semantics crystal clear.  What do you think?

Good suggestion. Thanks.

-Peff
--
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


Re: [PATCH 5/6] Git.pm: teach "ident" to query explicitness

2012-11-14 Thread Jonathan Nieder
Jeff King wrote:

> "git var" recently learned to report on whether an ident we
> fetch from it was configured explicitly or implicitly. Let's
> make that information available to callers of the ident
> function.

Sounds sensible.  Quick nits:

[...]
> --- a/perl/Git.pm
> +++ b/perl/Git.pm
> @@ -737,7 +737,7 @@ sub remote_refs {
>  }
>  
>  
> -=item ident ( TYPE | IDENTSTR )
> +=item ident ( TYPE | IDENTSTR [, options] )
>  
>  =item ident_person ( TYPE | IDENTSTR | IDENTARRAY )
>  
> @@ -750,6 +750,10 @@ and either returns it as a scalar string or as an array 
> with the fields parsed.
>  Alternatively, it can take a prepared ident string (e.g. from the commit
>  object) and just parse it.
>  
> +If the C option is set to 1, the returned array will contain an
> +additional boolean specifying whether the ident was configure explicitly by 
> the
> +user.

s/configure/configured/

I'd suggest adding "See GIT_COMMITTER_EXPLICIT in git-var(1) for
details" to make the semantics crystal clear.  What do you think?

Thanks,
Jonathan
--
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


Re: [PATCH 5/6] Git.pm: teach "ident" to query explicitness

2012-11-13 Thread Jeff King
On Tue, Nov 13, 2012 at 09:23:00AM -0800, Matt Kraai wrote:

> Minor nits:
> 
> On Tue, Nov 13, 2012 at 11:53:20AM -0500, Jeff King wrote:
> > @@ -750,6 +750,10 @@ and either returns it as a scalar string or as an 
> > array with the fields parsed.
> >  Alternatively, it can take a prepared ident string (e.g. from the commit
> >  object) and just parse it.
> >  
> > +If the C option is set to 1, the returned array will contain an
> > +additional boolean specifying whether the ident was configure explicitly 
> > by the
> 
> s/configure/configured/

Thanks.

> > if (wantarray) {
> > -   return $identstr =~ /^(.*) <(.*)> (\d+ [+-]\d{4})$/;
> > +   my @ret = $identstr =~ /^(.*) <(.*)> (\d+ [+-]\d{4})$/;
> > +   if ($options{explicit} && defined $explicit) {
> > +   push @ret, $explicit if defined $explicit;
> 
> Isn't the test on this line redundant given that defined $explicit is
> already guaranteed by the condition on the previous line?

Yes, thanks (left over from an earlier attempt that tried to avoid
having $options{explicit}).

-Peff
--
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


[PATCH 5/6] Git.pm: teach "ident" to query explicitness

2012-11-13 Thread Jeff King
"git var" recently learned to report on whether an ident we
fetch from it was configured explicitly or implicitly. Let's
make that information available to callers of the ident
function.

Because evaluating "ident" in an array versus scalar context
already has a meaning, we cannot return our extra value in a
backwards compatible way. Instead, we require the caller to
add an extra "explicit" flag to request the information.
The ident_person function, on the other hand, always returns
a scalar, so we are free to overload it in an array context.

Signed-off-by: Jeff King 
---
 perl/Git.pm | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/perl/Git.pm b/perl/Git.pm
index 497f420..1994ec1 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -737,7 +737,7 @@ sub remote_refs {
 }
 
 
-=item ident ( TYPE | IDENTSTR )
+=item ident ( TYPE | IDENTSTR [, options] )
 
 =item ident_person ( TYPE | IDENTSTR | IDENTARRAY )
 
@@ -750,6 +750,10 @@ and either returns it as a scalar string or as an array 
with the fields parsed.
 Alternatively, it can take a prepared ident string (e.g. from the commit
 object) and just parse it.
 
+If the C option is set to 1, the returned array will contain an
+additional boolean specifying whether the ident was configure explicitly by the
+user.
+
 C returns the person part of the ident - name and email;
 it can take the same arguments as C or the array returned by C.
 
@@ -763,17 +767,22 @@ The synopsis is like:
 =cut
 
 sub ident {
-   my ($self, $type) = _maybe_self(@_);
-   my $identstr;
+   my ($self, $type, %options) = _maybe_self(@_);
+   my ($identstr, $explicit);
if (lc $type eq lc 'committer' or lc $type eq lc 'author') {
-   my @cmd = ('var', 'GIT_'.uc($type).'_IDENT');
+   my $uc = uc($type);
+   my @cmd = ('var', "GIT_${uc}_IDENT", "GIT_${uc}_EXPLICIT");
unshift @cmd, $self if $self;
-   $identstr = command_oneline(@cmd);
+   ($identstr, $explicit) = command(@cmd);
} else {
$identstr = $type;
}
if (wantarray) {
-   return $identstr =~ /^(.*) <(.*)> (\d+ [+-]\d{4})$/;
+   my @ret = $identstr =~ /^(.*) <(.*)> (\d+ [+-]\d{4})$/;
+   if ($options{explicit} && defined $explicit) {
+   push @ret, $explicit if defined $explicit;
+   }
+   return @ret;
} else {
return $identstr;
}
@@ -781,8 +790,11 @@ sub ident {
 
 sub ident_person {
my ($self, @ident) = _maybe_self(@_);
-   $#ident == 0 and @ident = $self ? $self->ident($ident[0]) : 
ident($ident[0]);
-   return "$ident[0] <$ident[1]>";
+   $#ident == 0 and @ident = $self ?
+ $self->ident($ident[0], explicit => 1) :
+ ident($ident[0], explicit => 1);
+   my $ret = "$ident[0] <$ident[1]>";
+   return wantarray ? ($ret, @ident[3]) : $ret;
 }
 
 
-- 
1.8.0.207.gdf2154c

--
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