Re: [PATCH 2/2] git-credential-netrc: accept gpg option

[Luis Marsano]

Junio C Hamano  wrote:

> Luis Marsano  writes:
> 
> > git-credential-netrc was hardcoded to decrypt with 'gpg' regardless of the 
> > gpg.program option
> > this now uses the gpg command option if set, else, the gpg.program option 
> > set in the git repository or global configuration, else defaults to 'gpg'
> > for git-credential-netrc
> 
> These lines are way overlong.  Wrap at around 72-78 cols, perhaps.
> Complete each sentence with a full-stop.

Thanks, corrected this in the updated patch 
https://public-inbox.org/git/20180512091728.4931-3-luis.mars...@gmail.com/.

> > - use Git.pm for repository and global option queries
> > - add -g|--gpg command option & document it in command usage
> > - test repository & command options
> > - support unicode
> 
> There are other changes that are not explained/justified here, I
> think.
> 
>  - Instead of ALLCAPS as a placeholder for a command line argument in
>the help text, use , because doing so is better due
>to such and such reasons.
> 
> I think it is good to consistently do so, but it is unclear why
> ALLCAPS is bad and  is better.  That needs to be
> explained.

Not necessarily bad, but the reason was to conform with 
Documentation/CodingGuidelines.
The updated commit message now explains this.

>  - Replace three-dots in the help text with U+2026 to punish those
>who are still using unicode-inapable terminal in this century.
> 
> I do not think this part of the patch is a good idea at all, but
> perhaps I misunderstood the reason behind this change you had in
> mind (as you did not explain it in the proposed log message).

The original intent for this was semantics & accessibility: screenreaders more 
reliably interpret … than ... as ellipsis, and I imagine other assistive 
technology would, too.
However, after research, I've learned there are better supported ways to go 
about it, so I'm retracting that change.

The updated patch 
https://public-inbox.org/git/20180512091728.4931-3-luis.mars...@gmail.com/ 
should reflect all corrections.
Thank you for the feedback, and please let me know of further issues.

Re: [PATCH 2/2] git-credential-netrc: accept gpg option

[Junio C Hamano]

Luis Marsano  writes:

> git-credential-netrc was hardcoded to decrypt with 'gpg' regardless of the 
> gpg.program option
> this now uses the gpg command option if set, else, the gpg.program option set 
> in the git repository or global configuration, else defaults to 'gpg'
> for git-credential-netrc

These lines are way overlong.  Wrap at around 72-78 cols, perhaps.
Complete each sentence with a full-stop.

> - use Git.pm for repository and global option queries
> - add -g|--gpg command option & document it in command usage
> - test repository & command options
> - support unicode

There are other changes that are not explained/justified here, I
think.

 - Instead of ALLCAPS as a placeholder for a command line argument in
   the help text, use , because doing so is better due
   to such and such reasons.

I think it is good to consistently do so, but it is unclear why
ALLCAPS is bad and  is better.  That needs to be
explained.

 - Replace three-dots in the help text with U+2026 to punish those
   who are still using unicode-inapable terminal in this century.

I do not think this part of the patch is a good idea at all, but
perhaps I misunderstood the reason behind this change you had in
mind (as you did not explain it in the proposed log message).

> @@ -62,27 +69,31 @@ if ($options{help}) {
>  
>   print <  
> -$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
> +$0 [(-f )…] [-g ] [-d] [-v] [-k] get

Is this a desired change, or unwanted change left in the patch by
accident?


> -...and if you want lots of debugging info:
> +…and if you want lots of debugging info:

Is this a desired change, or unwanted change left in the patch by
accident?

>  
>git config credential.helper '$shortname -f AUTHFILE -d'
>  
> -...or to see the files opened and data found:
> +…or to see the files opened and data found:
>  

Ditto.

>git config credential.helper '$shortname -f AUTHFILE -v'
>  
> -Only "get" mode is supported by this credential helper.  It opens every 
> AUTHFILE
> +Only "get" mode is supported by this credential helper.  It opens every 
> 
>  and looks for the first entry that matches the requested search criteria:
>  
>   'port|protocol':
> @@ -120,7 +131,7 @@ host=github.com
>  protocol=https
>  username=tzz
>  
> -this credential helper will look for the first entry in every AUTHFILE that
> +this credential helper will look for the first entry in every  that
>  matches
>  
>  machine github.com port https login tzz
> @@ -129,7 +140,7 @@ OR
>  
>  machine github.com protocol https login tzz
>  
> -OR... etc. acceptable tokens as listed above.  Any unknown tokens are
> +OR… etc. acceptable tokens as listed above.  Any unknown tokens are

Ditto.

>  # Credentials must get a parameter, so die if it's missing.
> -die "Syntax: $0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] get" unless defined $mode;
> +die "Syntax: $0 [(-f )…] [-d] get" unless defined $mode;

Ditto.

[PATCH 2/2] git-credential-netrc: accept gpg option

[Luis Marsano]

git-credential-netrc was hardcoded to decrypt with 'gpg' regardless of the 
gpg.program option
this now uses the gpg command option if set, else, the gpg.program option set 
in the git repository or global configuration, else defaults to 'gpg'
for git-credential-netrc
- use Git.pm for repository and global option queries
- add -g|--gpg command option & document it in command usage
- test repository & command options
- support unicode

Signed-off-by: Luis Marsano 
Acked-by: Ted Zlatanov 
---
 contrib/credential/netrc/git-credential-netrc | 69 +--
 .../netrc/t-git-credential-netrc.sh   |  2 +-
 .../credential/netrc/test.command-option-gpg  |  2 +
 contrib/credential/netrc/test.git-config-gpg  |  2 +
 contrib/credential/netrc/test.netrc.gpg   |  0
 contrib/credential/netrc/test.pl  | 13 
 6 files changed, 65 insertions(+), 23 deletions(-)
 create mode 100755 contrib/credential/netrc/test.command-option-gpg
 create mode 100755 contrib/credential/netrc/test.git-config-gpg
 create mode 100644 contrib/credential/netrc/test.netrc.gpg

diff --git a/contrib/credential/netrc/git-credential-netrc 
b/contrib/credential/netrc/git-credential-netrc
index 1571a7b26..67cd689e8 100755
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -2,11 +2,16 @@
 
 use strict;
 use warnings;
+use autodie;
+use utf8;
+use open ':encoding(UTF-8)';
+use feature qw(unicode_strings unicode_eval);
 
 use Getopt::Long;
 use File::Basename;
+use Git;
 
-my $VERSION = "0.1";
+my $VERSION = "0.2";
 
 my %options = (
   help => 0,
@@ -14,6 +19,7 @@ my %options = (
   verbose => 0,
   insecure => 0,
   file => [],
+  gpg => '',
 
   # identical token maps, e.g. host -> host, will be inserted later
   tmap => {
@@ -54,6 +60,7 @@ GetOptions(\%options,
"insecure|k",
"verbose|v",
"file|f=s@",
+   'gpg|g:s',
   );
 
 if ($options{help}) {
@@ -62,27 +69,31 @@ if ($options{help}) {
 
print <