[PATCH 0/2] Configurable GnuPG command for git-credential-netrc

[PATCH 0/2] Configurable GnuPG command for git-credential-netrc

[Luis Marsano]

Hi everyone,

Should `contrib/` area patches go here, too? Contributed Software
page
https://git.kernel.org/pub/scm/git/git.git/tree/contrib/README
says to forward them to "jc" (I'm guessing that's Junio), but I
think they're getting missed. Though the component author approves
(acknowledgement below), am I missing anything for submission?
Thanks.

> git-credential-netrc was hardcoded to call 'gpg' for decryption.
> This is a problem on distributions like Debian that call modern
> GnuPG something else, like 'gpg2'. This proposed update reuses
> git's gpg.program configuration to customize GnuPG's name. It
> introduces the -g|--gpg option on the git-credential-netrc
> command to allow alternatively setting it in git's
> credential.helper configuration. Since testing a git
> configuration involves a temporary repository as provided by
> git's main testing framework, a patch updating tests to reuse
> the main framework is included, too.

------- Forwarded message
Delivered-To: luis.marsano@gmail.com
Received: by 10.103.152.29 with SMTP id a29csp214132vse; Fri, 20 Apr 2018
 16:18:34 -0700 (PDT)
Return-Path: <ted@lifelogs.com>
In-Reply-To: <1524160133-8877-1-git-send-email-luis.marsano@gmail.com>
References: <1524160133-8877-1-git-send-email-luis.marsano@gmail.com>
From: Ted Zlatanov <tzz@lifelogs.com>
Date: Fri, 20 Apr 2018 19:18:32 -0400
Message-ID: <CAMdP0d=sNtgwDnfZz88_cmAt6VYMGOozOC5q4o32R+yw+UcrAQ@mail.gmail.com>
Subject: Re: [RFC PATCH 0/2] Configurable GnuPG command for
 git-credential-netrc
To: Luis Marsano <luis.marsano@gmail.com>
Status: OR

That looks terrific, Luis. It's a good change so I'd just go ahead
and propose those patches. I don't see anything incorrect and
tests are always good.

Ted

On Thu, Apr 19, 2018 at 1:48 PM, Luis Marsano luis.marsano@gmail.com
wrote:

> Hello Ted Zlatanov,

> Thank you for git-credential-netrc. Will you please look over
> these patches and decide if they need more work? They're also
> available as the netrc-option branch at
> https://github.com/lmmarsano/git This is a first contribution
> for me, and I welcome input, suggestions, criticisms,
> corrections. Thanks.

> git-credential-netrc was hardcoded to call 'gpg' for decryption.
> This is a problem on distributions like Debian that call modern
> GnuPG something else, like 'gpg2'. This proposed update reuses
> git's gpg.program configuration to customize GnuPG's name. It
> introduces the -g|--gpg option on the git-credential-netrc
> command to allow alternatively setting it in git's
> credential.helper configuration. Since testing a git
> configuration involves a temporary repository as provided by
> git's main testing framework, a patch updating tests to reuse
> the main framework is included, too.

> Luis Marsano (2):
> git-credential-netrc: adapt to test framework for git
> git-credential-netrc: accept gpg option

> contrib/credential/netrc/Makefile | 4 +-
> contrib/credential/netrc/git-credential-netrc | 69 +++++++++++------
> contrib/credential/netrc/t-git-credential-netrc.sh | 31 ++++++++
> contrib/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 | 86
> +++++++++++++++-------
> 7 files changed, 142 insertions(+), 52 deletions(-)
> create mode 100755 contrib/credential/netrc/t-git-credential-netrc.sh
> 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

> --
> 2.7.4

------- End of Forwarded message

Luis Marsano (2):
  git-credential-netrc: adapt to test framework for git
  git-credential-netrc: accept gpg option

 contrib/credential/netrc/Makefile             |  4 +-
 contrib/credential/netrc/git-credential-netrc | 69 ++++++++++-----
 .../netrc/t-git-credential-netrc.sh           | 31 +++++++
 .../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              | 86 +++++++++++++------
 7 files changed, 142 insertions(+), 52 deletions(-)
 create mode 100755 contrib/credential/netrc/t-git-credential-netrc.sh
 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

-- 
2.17.0

[PATCH 1/2] git-credential-netrc: adapt to test framework for git

[Luis Marsano]

until this change, git-credential-netrc did not test in a repository
this change reuses the main test framework, which provides such tests
specific changes
- switch to Test::More module
- use File::Basename & File::Spec::Functions

Signed-off-by: Luis Marsano <luis.marsano@gmail.com>
Acked-by: Ted Zlatanov <tzz@lifelogs.com>
---
 contrib/credential/netrc/Makefile             |  4 +-
 .../netrc/t-git-credential-netrc.sh           | 31 ++++++++
 contrib/credential/netrc/test.pl              | 73 ++++++++++++-------
 3 files changed, 78 insertions(+), 30 deletions(-)
 create mode 100755 contrib/credential/netrc/t-git-credential-netrc.sh

diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
index 51b76138a..0ffa40719 100644
--- a/contrib/credential/netrc/Makefile
+++ b/contrib/credential/netrc/Makefile
@@ -1,5 +1,5 @@
 test:
-	./test.pl
+	./t-git-credential-netrc.sh
 
 testverbose:
-	./test.pl -d -v
+	./t-git-credential-netrc.sh -d -v
diff --git a/contrib/credential/netrc/t-git-credential-netrc.sh b/contrib/credential/netrc/t-git-credential-netrc.sh
new file mode 100755
index 000000000..fa21ca240
--- /dev/null
+++ b/contrib/credential/netrc/t-git-credential-netrc.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+(
+	cd ../../../t
+	test_description='git-credential-netrc'
+	. ./test-lib.sh
+
+	if ! test_have_prereq PERL; then
+		skip_all='skipping perl interface tests, perl not available'
+		test_done
+	fi
+
+	perl -MTest::More -e 0 2>/dev/null || {
+		skip_all="Perl Test::More unavailable, skipping test"
+		test_done
+	}
+
+	# set up test repository
+
+	test_expect_success \
+    'set up test repository' \
+    ':'
+
+	# The external test will outputs its own plan
+	test_external_has_tap=1
+
+	test_external \
+    'git-credential-netrc' \
+    perl "$TEST_DIRECTORY"/../contrib/credential/netrc/test.pl
+
+	test_done
+)
diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl
index 169b6463c..7211b4857 100755
--- a/contrib/credential/netrc/test.pl
+++ b/contrib/credential/netrc/test.pl
@@ -1,83 +1,100 @@
 #!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
 
 use warnings;
 use strict;
-use Test;
+use Test::More qw(no_plan);
+use File::Basename;
+use File::Spec::Functions qw(:DEFAULT rel2abs);
 use IPC::Open2;
 
-BEGIN { plan tests => 15 }
+BEGIN {
+	# t-git-credential-netrc.sh kicks off our testing, so we have to go from there.
+	Test::More->builder->current_test(1);
+	Test::More->builder->no_ending(1);
+}
 
 my @global_credential_args = @ARGV;
-my $netrc = './test.netrc';
-print "# Testing insecure file, nothing should be found\n";
+my $scriptDir = dirname rel2abs $0;
+my $netrc = catfile $scriptDir, 'test.netrc';
+my $netrcGpg = catfile $scriptDir, 'test.netrc.gpg';
+my $gcNetrc = catfile $scriptDir, 'git-credential-netrc';
+local $ENV{PATH} = join ':'
+                      , $scriptDir
+                      , $ENV{PATH}
+                      ? $ENV{PATH}
+                      : ();
+
+diag "Testing insecure file, nothing should be found\n";
 chmod 0644, $netrc;
 my $cred = run_credential(['-f', $netrc, 'get'],
 			  { host => 'github.com' });
 
-ok(scalar keys %$cred, 0, "Got 0 keys from insecure file");
+ok(scalar keys %$cred == 0, "Got 0 keys from insecure file");
 
-print "# Testing missing file, nothing should be found\n";
+diag "Testing missing file, nothing should be found\n";
 chmod 0644, $netrc;
 $cred = run_credential(['-f', '///nosuchfile///', 'get'],
 		       { host => 'github.com' });
 
-ok(scalar keys %$cred, 0, "Got 0 keys from missing file");
+ok(scalar keys %$cred == 0, "Got 0 keys from missing file");
 
 chmod 0600, $netrc;
 
-print "# Testing with invalid data\n";
+diag "Testing with invalid data\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       "bad data");
-ok(scalar keys %$cred, 4, "Got first found keys with bad data");
+ok(scalar keys %$cred == 4, "Got first found keys with bad data");
 
-print "# Testing netrc file for a missing corovamilkbar entry\n";
+diag "Testing netrc file for a missing corovamilkbar entry\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       { host => 'corovamilkbar' });
 
-ok(scalar keys %$cred, 0, "Got no corovamilkbar keys");
+ok(scalar keys %$cred == 0, "Got no corovamilkbar keys");
 
-print "# Testing netrc file for a github.com entry\n";
+diag "Testing netrc file for a github.com entry\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       { host => 'github.com' });
 
-ok(scalar keys %$cred, 2, "Got 2 Github keys");
+ok(scalar keys %$cred == 2, "Got 2 Github keys");
 
-ok($cred->{password}, 'carolknows', "Got correct Github password");
-ok($cred->{username}, 'carol', "Got correct Github username");
+is($cred->{password}, 'carolknows', "Got correct Github password");
+is($cred->{username}, 'carol', "Got correct Github username");
 
-print "# Testing netrc file for a username-specific entry\n";
+diag "Testing netrc file for a username-specific entry\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       { host => 'imap', username => 'bob' });
 
-ok(scalar keys %$cred, 2, "Got 2 username-specific keys");
+ok(scalar keys %$cred == 2, "Got 2 username-specific keys");
 
-ok($cred->{password}, 'bobwillknow', "Got correct user-specific password");
-ok($cred->{protocol}, 'imaps', "Got correct user-specific protocol");
+is($cred->{password}, 'bobwillknow', "Got correct user-specific password");
+is($cred->{protocol}, 'imaps', "Got correct user-specific protocol");
 
-print "# Testing netrc file for a host:port-specific entry\n";
+diag "Testing netrc file for a host:port-specific entry\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       { host => 'imap2:1099' });
 
-ok(scalar keys %$cred, 2, "Got 2 host:port-specific keys");
+ok(scalar keys %$cred == 2, "Got 2 host:port-specific keys");
 
-ok($cred->{password}, 'tzzknow', "Got correct host:port-specific password");
-ok($cred->{username}, 'tzz', "Got correct host:port-specific username");
+is($cred->{password}, 'tzzknow', "Got correct host:port-specific password");
+is($cred->{username}, 'tzz', "Got correct host:port-specific username");
 
-print "# Testing netrc file that 'host:port kills host' entry\n";
+diag "Testing netrc file that 'host:port kills host' entry\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       { host => 'imap2' });
 
-ok(scalar keys %$cred, 2, "Got 2 'host:port kills host' keys");
+ok(scalar keys %$cred == 2, "Got 2 'host:port kills host' keys");
+
+is($cred->{password}, 'bobwillknow', "Got correct 'host:port kills host' password");
+is($cred->{username}, 'bob', "Got correct 'host:port kills host' username");
 
-ok($cred->{password}, 'bobwillknow', "Got correct 'host:port kills host' password");
-ok($cred->{username}, 'bob', "Got correct 'host:port kills host' username");
 
 sub run_credential
 {
 	my $args = shift @_;
 	my $data = shift @_;
 	my $pid = open2(my $chld_out, my $chld_in,
-			'./git-credential-netrc', @global_credential_args,
+			$gcNetrc, @global_credential_args,
 			@$args);
 
 	die "Couldn't open pipe to netrc credential helper: $!" unless $pid;
-- 
2.17.0

[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 <luis.marsano@gmail.com>
Acked-by: Ted Zlatanov <tzz@lifelogs.com>
---
 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 <<EOHIPPUS;
 
-$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
+$0 [(-f <authfile>)…] [-g <program>] [-d] [-v] [-k] get
 
 Version $VERSION by tzz\@lifelogs.com.  License: BSD.
 
 Options:
 
-  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg extension
-                       will be decrypted by GPG before parsing.  Multiple -f
-                       arguments are OK.  They are processed in order, and the
-                       first matching entry found is returned via the credential
-                       helper protocol (see below).
+  -f|--file <authfile>: specify netrc-style files.  Files with the .gpg extension
+                        will be decrypted by GPG before parsing.  Multiple -f
+                        arguments are OK.  They are processed in order, and the
+                        first matching entry found is returned via the credential
+                        helper protocol (see below).
 
-                       When no -f option is given, .authinfo.gpg, .netrc.gpg,
-		       .authinfo, and .netrc files in your home directory are used
-		       in this order.
+                        When no -f option is given, .authinfo.gpg, .netrc.gpg,
+                        .authinfo, and .netrc files in your home directory are used
+                        in this order.
 
-  -k|--insecure      : ignore bad file ownership or permissions
+  -g|--gpg <program>  : specify the program for GPG. By default, this is the value
+                        of gpg.program in the git repository or global option or
+                        gpg.
 
-  -d|--debug         : turn on debugging (developer info)
+  -k|--insecure       : ignore bad file ownership or permissions
 
-  -v|--verbose       : be more verbose (show files and information found)
+  -d|--debug          : turn on debugging (developer info)
+
+  -v|--verbose        : be more verbose (show files and information found)
 
 To enable this credential helper:
 
@@ -91,15 +102,15 @@ To enable this credential helper:
 (Note that Git will prepend "git-credential-" to the helper name and look for it
 in the path.)
 
-...and if you want lots of debugging info:
+…and if you want lots of debugging info:
 
   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:
 
   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 <authfile>
 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 <authfile> 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
 simply ignored.
 
 Then, the helper will print out whatever tokens it got from the entry, including
@@ -137,7 +148,7 @@ Then, the helper will print out whatever tokens it got from the entry, including
 back to "protocol".  Any redundant entry tokens (part of the original query) are
 skipped.
 
-Again, note that only the first matching entry from all the AUTHFILEs, processed
+Again, note that only the first matching entry from all the <authfile>s, processed
 in the sequence given on the command line, is used.
 
 Netrc/authinfo tokens can be quoted as 'STRING' or "STRING".
@@ -152,7 +163,7 @@ EOHIPPUS
 my $mode = shift @ARGV;
 
 # 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 <authfile>)…] [-d] get" unless defined $mode;
 
 # Only support 'get' mode; with any other unsupported ones we just exit.
 exit 0 unless $mode eq 'get';
@@ -172,6 +183,8 @@ unless (scalar @$files) {
 	$files = $options{file} = [ map { glob $_ } @candidates ];
 }
 
+load_config(\%options);
+
 my $query = read_credential_data_from_stdin();
 
 FILE:
@@ -233,11 +246,11 @@ sub load_netrc {
 
 	my $io;
 	if ($gpgmode) {
-		my @cmd = (qw(gpg --decrypt), $file);
+		my @cmd = ($options{'gpg'}, qw(--decrypt), $file);
 		log_verbose("Using GPG to open $file: [@cmd]");
 		open $io, "-|", @cmd;
 	} else {
-		log_verbose("Opening $file...");
+		log_verbose("Opening ${file}…");
 		open $io, '<', $file;
 	}
 
@@ -410,6 +423,18 @@ sub print_credential_data {
 		printf "%s=%s\n", $git_token, $entry->{$git_token};
 	}
 }
+sub load_config {
+	# load settings from git config
+	my $options = shift;
+	if ($options->{'gpg'} eq '') {
+		local $_ = Git->repository()->config('gpg.program');
+		# default to gpg for non-0 exit code
+		$options->{'gpg'} = defined
+		                 ? $_
+		                 : 'gpg';
+	}
+	log_verbose("using $options{'gpg'} for GPG operations");
+}
 sub log_verbose {
 	return unless $options{verbose};
 	printf STDERR @_;
diff --git a/contrib/credential/netrc/t-git-credential-netrc.sh b/contrib/credential/netrc/t-git-credential-netrc.sh
index fa21ca240..58191a62f 100755
--- a/contrib/credential/netrc/t-git-credential-netrc.sh
+++ b/contrib/credential/netrc/t-git-credential-netrc.sh
@@ -18,7 +18,7 @@
 
 	test_expect_success \
     'set up test repository' \
-    ':'
+    'git config --add gpg.program test.git-config-gpg'
 
 	# The external test will outputs its own plan
 	test_external_has_tap=1
diff --git a/contrib/credential/netrc/test.command-option-gpg b/contrib/credential/netrc/test.command-option-gpg
new file mode 100755
index 000000000..d8f1285d4
--- /dev/null
+++ b/contrib/credential/netrc/test.command-option-gpg
@@ -0,0 +1,2 @@
+#!/bin/sh
+echo machine command-option-gpg login username password password
diff --git a/contrib/credential/netrc/test.git-config-gpg b/contrib/credential/netrc/test.git-config-gpg
new file mode 100755
index 000000000..65cf594c2
--- /dev/null
+++ b/contrib/credential/netrc/test.git-config-gpg
@@ -0,0 +1,2 @@
+#!/bin/sh
+echo machine git-config-gpg login username password password
diff --git a/contrib/credential/netrc/test.netrc.gpg b/contrib/credential/netrc/test.netrc.gpg
new file mode 100644
index 000000000..e69de29bb
diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl
index 7211b4857..3ec8934ac 100755
--- a/contrib/credential/netrc/test.pl
+++ b/contrib/credential/netrc/test.pl
@@ -88,6 +88,19 @@ ok(scalar keys %$cred == 2, "Got 2 'host:port kills host' keys");
 is($cred->{password}, 'bobwillknow', "Got correct 'host:port kills host' password");
 is($cred->{username}, 'bob', "Got correct 'host:port kills host' username");
 
+diag 'Testing netrc file decryption by git config gpg.program setting\n';
+$cred = run_credential( ['-f', $netrcGpg, 'get']
+                      , { host => 'git-config-gpg' }
+                      );
+
+ok(scalar keys %$cred == 2, 'Got keys decrypted by git config option');
+
+diag 'Testing netrc file decryption by gpg option\n';
+$cred = run_credential( ['-f', $netrcGpg, '-g', 'test.command-option-gpg', 'get']
+                      , { host => 'command-option-gpg' }
+                      );
+
+ok(scalar keys %$cred == 2, 'Got keys decrypted by command option');
 
 sub run_credential
 {
-- 
2.17.0

Re: [PATCH 1/2] git-credential-netrc: adapt to test framework for git

[Junio C Hamano]

Luis Marsano <luis.marsano@gmail.com> writes:

> until this change, git-credential-netrc did not test in a repository
> this change reuses the main test framework, which provides such tests
> specific changes

Sorry, but I cannot quite parse what the above is trying to say.

> - switch to Test::More module
> - use File::Basename & File::Spec::Functions
>
> Signed-off-by: Luis Marsano <luis.marsano@gmail.com>
> Acked-by: Ted Zlatanov <tzz@lifelogs.com>
> ---
>  contrib/credential/netrc/Makefile             |  4 +-
>  .../netrc/t-git-credential-netrc.sh           | 31 ++++++++
>  contrib/credential/netrc/test.pl              | 73 ++++++++++++-------
>  3 files changed, 78 insertions(+), 30 deletions(-)
>  create mode 100755 contrib/credential/netrc/t-git-credential-netrc.sh

Will queue, but may need to make the log message
readable/understandable.

Thanks.

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

[Junio C Hamano]

Luis Marsano <luis.marsano@gmail.com> 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 <placeholder>, 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 <placeholder> 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 <<EOHIPPUS;
>  
> -$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
> +$0 [(-f <authfile>)…] [-g <program>] [-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 <authfile>
>  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 <authfile> 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 <authfile>)…] [-d] get" unless defined $mode;

Ditto.

[PATCH v2 0/2] Configurable GnuPG command for git-credential-netrc

[Luis Marsano]

Updated per Junio's comments.
- clarify commit messages
- explain usage placeholder style as conformance with main coding style
- drop Unicode dependence

Unicode punctuation was initially included for semantics & accessibility, since assistive technology like screenreaders handle … better than ...
However, character conversion between encodings is typically handled through Native Language Support such as gettext, which recommends untranslated strings in ASCII.
Therefore, strings remain in ASCII to ease extensibility in that direction.

The following changes since commit ccdcbd54c4475c2238b310f7113ab3075b5abc9c:

  The fifth batch for 2.18 (2018-05-08 15:59:49 +0900)

are available in the Git repository at:

  https://github.com/lmmarsano/git.git 

for you to fetch changes up to 62a76a1eb7906199b731858ae2d193cfdd3176e0:

  git-credential-netrc: accept gpg option (2018-05-12 03:27:45 -0400)

----------------------------------------------------------------
Luis Marsano (2):
  git-credential-netrc: adapt to test framework for git
  git-credential-netrc: accept gpg option

 contrib/credential/netrc/Makefile             |  4 +-
 contrib/credential/netrc/git-credential-netrc | 58 +++++++-----
 .../netrc/t-git-credential-netrc.sh           | 31 +++++++
 .../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              | 88 +++++++++++++------
 7 files changed, 135 insertions(+), 50 deletions(-)
 create mode 100755 contrib/credential/netrc/t-git-credential-netrc.sh
 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

-- 
2.17.0

[PATCH v2 1/2] git-credential-netrc: adapt to test framework for git

[Luis Marsano]

git-credential-netrc tests did not run in a test repository.
Reuse the main test framework to stage a temporary repository.
To imitate Perl tests under t/
- switch to Test::More module
- use File::Basename & File::Spec::Functions

Signed-off-by: Luis Marsano <luis.marsano@gmail.com>
Acked-by: Ted Zlatanov <tzz@lifelogs.com>
---
 contrib/credential/netrc/Makefile             |  4 +-
 .../netrc/t-git-credential-netrc.sh           | 31 ++++++++
 contrib/credential/netrc/test.pl              | 72 +++++++++++--------
 3 files changed, 77 insertions(+), 30 deletions(-)
 create mode 100755 contrib/credential/netrc/t-git-credential-netrc.sh

diff --git a/contrib/credential/netrc/Makefile b/contrib/credential/netrc/Makefile
index 51b76138a..0ffa40719 100644
--- a/contrib/credential/netrc/Makefile
+++ b/contrib/credential/netrc/Makefile
@@ -1,5 +1,5 @@
 test:
-	./test.pl
+	./t-git-credential-netrc.sh
 
 testverbose:
-	./test.pl -d -v
+	./t-git-credential-netrc.sh -d -v
diff --git a/contrib/credential/netrc/t-git-credential-netrc.sh b/contrib/credential/netrc/t-git-credential-netrc.sh
new file mode 100755
index 000000000..efb81efed
--- /dev/null
+++ b/contrib/credential/netrc/t-git-credential-netrc.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+(
+	cd ../../../t
+	test_description='git-credential-netrc'
+	. ./test-lib.sh
+
+	if ! test_have_prereq PERL; then
+		skip_all='skipping perl interface tests, perl not available'
+		test_done
+	fi
+
+	perl -MTest::More -e 0 2>/dev/null || {
+		skip_all="Perl Test::More unavailable, skipping test"
+		test_done
+	}
+
+	# set up test repository
+
+	test_expect_success \
+    'set up test repository' \
+    :
+
+	# The external test will outputs its own plan
+	test_external_has_tap=1
+
+	test_external \
+    'git-credential-netrc' \
+    perl "$TEST_DIRECTORY"/../contrib/credential/netrc/test.pl
+
+	test_done
+)
diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl
index 169b6463c..ea95a2c8a 100755
--- a/contrib/credential/netrc/test.pl
+++ b/contrib/credential/netrc/test.pl
@@ -1,83 +1,99 @@
 #!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
 
 use warnings;
 use strict;
-use Test;
+use Test::More qw(no_plan);
+use File::Basename;
+use File::Spec::Functions qw(:DEFAULT rel2abs);
 use IPC::Open2;
 
-BEGIN { plan tests => 15 }
+BEGIN {
+	# t-git-credential-netrc.sh kicks off our testing, so we have to go from there.
+	Test::More->builder->current_test(1);
+	Test::More->builder->no_ending(1);
+}
 
 my @global_credential_args = @ARGV;
-my $netrc = './test.netrc';
-print "# Testing insecure file, nothing should be found\n";
+my $scriptDir = dirname rel2abs $0;
+my $netrc = catfile $scriptDir, 'test.netrc';
+my $gcNetrc = catfile $scriptDir, 'git-credential-netrc';
+local $ENV{PATH} = join ':'
+                      , $scriptDir
+                      , $ENV{PATH}
+                      ? $ENV{PATH}
+                      : ();
+
+diag "Testing insecure file, nothing should be found\n";
 chmod 0644, $netrc;
 my $cred = run_credential(['-f', $netrc, 'get'],
 			  { host => 'github.com' });
 
-ok(scalar keys %$cred, 0, "Got 0 keys from insecure file");
+ok(scalar keys %$cred == 0, "Got 0 keys from insecure file");
 
-print "# Testing missing file, nothing should be found\n";
+diag "Testing missing file, nothing should be found\n";
 chmod 0644, $netrc;
 $cred = run_credential(['-f', '///nosuchfile///', 'get'],
 		       { host => 'github.com' });
 
-ok(scalar keys %$cred, 0, "Got 0 keys from missing file");
+ok(scalar keys %$cred == 0, "Got 0 keys from missing file");
 
 chmod 0600, $netrc;
 
-print "# Testing with invalid data\n";
+diag "Testing with invalid data\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       "bad data");
-ok(scalar keys %$cred, 4, "Got first found keys with bad data");
+ok(scalar keys %$cred == 4, "Got first found keys with bad data");
 
-print "# Testing netrc file for a missing corovamilkbar entry\n";
+diag "Testing netrc file for a missing corovamilkbar entry\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       { host => 'corovamilkbar' });
 
-ok(scalar keys %$cred, 0, "Got no corovamilkbar keys");
+ok(scalar keys %$cred == 0, "Got no corovamilkbar keys");
 
-print "# Testing netrc file for a github.com entry\n";
+diag "Testing netrc file for a github.com entry\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       { host => 'github.com' });
 
-ok(scalar keys %$cred, 2, "Got 2 Github keys");
+ok(scalar keys %$cred == 2, "Got 2 Github keys");
 
-ok($cred->{password}, 'carolknows', "Got correct Github password");
-ok($cred->{username}, 'carol', "Got correct Github username");
+is($cred->{password}, 'carolknows', "Got correct Github password");
+is($cred->{username}, 'carol', "Got correct Github username");
 
-print "# Testing netrc file for a username-specific entry\n";
+diag "Testing netrc file for a username-specific entry\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       { host => 'imap', username => 'bob' });
 
-ok(scalar keys %$cred, 2, "Got 2 username-specific keys");
+ok(scalar keys %$cred == 2, "Got 2 username-specific keys");
 
-ok($cred->{password}, 'bobwillknow', "Got correct user-specific password");
-ok($cred->{protocol}, 'imaps', "Got correct user-specific protocol");
+is($cred->{password}, 'bobwillknow', "Got correct user-specific password");
+is($cred->{protocol}, 'imaps', "Got correct user-specific protocol");
 
-print "# Testing netrc file for a host:port-specific entry\n";
+diag "Testing netrc file for a host:port-specific entry\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       { host => 'imap2:1099' });
 
-ok(scalar keys %$cred, 2, "Got 2 host:port-specific keys");
+ok(scalar keys %$cred == 2, "Got 2 host:port-specific keys");
 
-ok($cred->{password}, 'tzzknow', "Got correct host:port-specific password");
-ok($cred->{username}, 'tzz', "Got correct host:port-specific username");
+is($cred->{password}, 'tzzknow', "Got correct host:port-specific password");
+is($cred->{username}, 'tzz', "Got correct host:port-specific username");
 
-print "# Testing netrc file that 'host:port kills host' entry\n";
+diag "Testing netrc file that 'host:port kills host' entry\n";
 $cred = run_credential(['-f', $netrc, 'get'],
 		       { host => 'imap2' });
 
-ok(scalar keys %$cred, 2, "Got 2 'host:port kills host' keys");
+ok(scalar keys %$cred == 2, "Got 2 'host:port kills host' keys");
+
+is($cred->{password}, 'bobwillknow', "Got correct 'host:port kills host' password");
+is($cred->{username}, 'bob', "Got correct 'host:port kills host' username");
 
-ok($cred->{password}, 'bobwillknow', "Got correct 'host:port kills host' password");
-ok($cred->{username}, 'bob', "Got correct 'host:port kills host' username");
 
 sub run_credential
 {
 	my $args = shift @_;
 	my $data = shift @_;
 	my $pid = open2(my $chld_out, my $chld_in,
-			'./git-credential-netrc', @global_credential_args,
+			$gcNetrc, @global_credential_args,
 			@$args);
 
 	die "Couldn't open pipe to netrc credential helper: $!" unless $pid;
-- 
2.17.0

[PATCH v2 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 is a problem on distributions like Debian
that call modern GnuPG something else, like 'gpg2'.
Set the command according to these settings in descending precedence
1. the git-credential-netrc command -g|--gpg option
2. the git gpg.program configuration option
3. the default: 'gpg'

For conformance with Documentation/CodingGuidelines
- use Git.pm for repository and global option queries
- document -g|--gpg command option in command usage
- test repository & command options
- write documentation placeholders according to main standards

Signed-off-by: Luis Marsano <luis.marsano@gmail.com>
Acked-by: Ted Zlatanov <tzz@lifelogs.com>
---
 contrib/credential/netrc/git-credential-netrc | 58 ++++++++++++-------
 .../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              | 22 ++++++-
 6 files changed, 62 insertions(+), 24 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..0b9a94102 100755
--- a/contrib/credential/netrc/git-credential-netrc
+++ b/contrib/credential/netrc/git-credential-netrc
@@ -2,11 +2,13 @@
 
 use strict;
 use warnings;
+use autodie;
 
 use Getopt::Long;
 use File::Basename;
+use Git;
 
-my $VERSION = "0.1";
+my $VERSION = "0.2";
 
 my %options = (
 	       help => 0,
@@ -54,6 +56,7 @@ GetOptions(\%options,
            "insecure|k",
            "verbose|v",
            "file|f=s@",
+           'gpg|g:s',
           );
 
 if ($options{help}) {
@@ -62,27 +65,31 @@ if ($options{help}) {
 
 	print <<EOHIPPUS;
 
-$0 [-f AUTHFILE1] [-f AUTHFILEN] [-d] [-v] [-k] get
+$0 [(-f <authfile>)...] [-g <program>] [-d] [-v] [-k] get
 
 Version $VERSION by tzz\@lifelogs.com.  License: BSD.
 
 Options:
 
-  -f|--file AUTHFILE : specify netrc-style files.  Files with the .gpg extension
-                       will be decrypted by GPG before parsing.  Multiple -f
-                       arguments are OK.  They are processed in order, and the
-                       first matching entry found is returned via the credential
-                       helper protocol (see below).
+  -f|--file <authfile>: specify netrc-style files.  Files with the .gpg
+                        extension will be decrypted by GPG before parsing.
+                        Multiple -f arguments are OK.  They are processed in
+                        order, and the first matching entry found is returned
+                        via the credential helper protocol (see below).
 
-                       When no -f option is given, .authinfo.gpg, .netrc.gpg,
-		       .authinfo, and .netrc files in your home directory are used
-		       in this order.
+                        When no -f option is given, .authinfo.gpg, .netrc.gpg,
+                        .authinfo, and .netrc files in your home directory are
+                        used in this order.
 
-  -k|--insecure      : ignore bad file ownership or permissions
+  -g|--gpg <program>  : specify the program for GPG. By default, this is the
+                        value of gpg.program in the git repository or global
+                        option or gpg.
 
-  -d|--debug         : turn on debugging (developer info)
+  -k|--insecure       : ignore bad file ownership or permissions
 
-  -v|--verbose       : be more verbose (show files and information found)
+  -d|--debug          : turn on debugging (developer info)
+
+  -v|--verbose        : be more verbose (show files and information found)
 
 To enable this credential helper:
 
@@ -99,8 +106,9 @@ in the path.)
 
   git config credential.helper '$shortname -f AUTHFILE -v'
 
-Only "get" mode is supported by this credential helper.  It opens every AUTHFILE
-and looks for the first entry that matches the requested search criteria:
+Only "get" mode is supported by this credential helper.  It opens every
+<authfile> and looks for the first entry that matches the requested search
+criteria:
 
  'port|protocol':
    The protocol that will be used (e.g., https). (protocol=X)
@@ -120,7 +128,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 <authfile> that
 matches
 
 machine github.com port https login tzz
@@ -137,8 +145,8 @@ Then, the helper will print out whatever tokens it got from the entry, including
 back to "protocol".  Any redundant entry tokens (part of the original query) are
 skipped.
 
-Again, note that only the first matching entry from all the AUTHFILEs, processed
-in the sequence given on the command line, is used.
+Again, note that only the first matching entry from all the <authfile>s,
+processed in the sequence given on the command line, is used.
 
 Netrc/authinfo tokens can be quoted as 'STRING' or "STRING".
 
@@ -152,7 +160,7 @@ EOHIPPUS
 my $mode = shift @ARGV;
 
 # 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 <authfile>)...] [-d] get" unless defined $mode;
 
 # Only support 'get' mode; with any other unsupported ones we just exit.
 exit 0 unless $mode eq 'get';
@@ -172,6 +180,8 @@ unless (scalar @$files) {
 	$files = $options{file} = [ map { glob $_ } @candidates ];
 }
 
+load_config(\%options);
+
 my $query = read_credential_data_from_stdin();
 
 FILE:
@@ -233,7 +243,7 @@ sub load_netrc {
 
 	my $io;
 	if ($gpgmode) {
-		my @cmd = (qw(gpg --decrypt), $file);
+		my @cmd = ($options{'gpg'}, qw(--decrypt), $file);
 		log_verbose("Using GPG to open $file: [@cmd]");
 		open $io, "-|", @cmd;
 	} else {
@@ -410,6 +420,14 @@ sub print_credential_data {
 		printf "%s=%s\n", $git_token, $entry->{$git_token};
 	}
 }
+sub load_config {
+	# load settings from git config
+	my $options = shift;
+	# set from command argument, gpg.program option, or default to gpg
+	$options->{'gpg'} //= Git->repository()->config('gpg.program')
+	                  // 'gpg';
+	log_verbose("using $options{'gpg'} for GPG operations");
+}
 sub log_verbose {
 	return unless $options{verbose};
 	printf STDERR @_;
diff --git a/contrib/credential/netrc/t-git-credential-netrc.sh b/contrib/credential/netrc/t-git-credential-netrc.sh
index efb81efed..58191a62f 100755
--- a/contrib/credential/netrc/t-git-credential-netrc.sh
+++ b/contrib/credential/netrc/t-git-credential-netrc.sh
@@ -18,7 +18,7 @@
 
 	test_expect_success \
     'set up test repository' \
-    :
+    'git config --add gpg.program test.git-config-gpg'
 
 	# The external test will outputs its own plan
 	test_external_has_tap=1
diff --git a/contrib/credential/netrc/test.command-option-gpg b/contrib/credential/netrc/test.command-option-gpg
new file mode 100755
index 000000000..d8f1285d4
--- /dev/null
+++ b/contrib/credential/netrc/test.command-option-gpg
@@ -0,0 +1,2 @@
+#!/bin/sh
+echo machine command-option-gpg login username password password
diff --git a/contrib/credential/netrc/test.git-config-gpg b/contrib/credential/netrc/test.git-config-gpg
new file mode 100755
index 000000000..65cf594c2
--- /dev/null
+++ b/contrib/credential/netrc/test.git-config-gpg
@@ -0,0 +1,2 @@
+#!/bin/sh
+echo machine git-config-gpg login username password password
diff --git a/contrib/credential/netrc/test.netrc.gpg b/contrib/credential/netrc/test.netrc.gpg
new file mode 100644
index 000000000..e69de29bb
diff --git a/contrib/credential/netrc/test.pl b/contrib/credential/netrc/test.pl
index ea95a2c8a..1e1001030 100755
--- a/contrib/credential/netrc/test.pl
+++ b/contrib/credential/netrc/test.pl
@@ -9,15 +9,18 @@ use File::Spec::Functions qw(:DEFAULT rel2abs);
 use IPC::Open2;
 
 BEGIN {
-	# t-git-credential-netrc.sh kicks off our testing, so we have to go from there.
+	# t-git-credential-netrc.sh kicks off our testing, so we have to go
+	# from there.
 	Test::More->builder->current_test(1);
 	Test::More->builder->no_ending(1);
 }
 
 my @global_credential_args = @ARGV;
 my $scriptDir = dirname rel2abs $0;
-my $netrc = catfile $scriptDir, 'test.netrc';
-my $gcNetrc = catfile $scriptDir, 'git-credential-netrc';
+my ($netrc, $netrcGpg, $gcNetrc) = map { catfile $scriptDir, $_; }
+                                       qw(test.netrc
+                                          test.netrc.gpg
+                                          git-credential-netrc);
 local $ENV{PATH} = join ':'
                       , $scriptDir
                       , $ENV{PATH}
@@ -87,6 +90,19 @@ ok(scalar keys %$cred == 2, "Got 2 'host:port kills host' keys");
 is($cred->{password}, 'bobwillknow', "Got correct 'host:port kills host' password");
 is($cred->{username}, 'bob', "Got correct 'host:port kills host' username");
 
+diag 'Testing netrc file decryption by git config gpg.program setting\n';
+$cred = run_credential( ['-f', $netrcGpg, 'get']
+                      , { host => 'git-config-gpg' }
+                      );
+
+ok(scalar keys %$cred == 2, 'Got keys decrypted by git config option');
+
+diag 'Testing netrc file decryption by gpg option\n';
+$cred = run_credential( ['-f', $netrcGpg, '-g', 'test.command-option-gpg', 'get']
+                      , { host => 'command-option-gpg' }
+                      );
+
+ok(scalar keys %$cred == 2, 'Got keys decrypted by command option');
 
 sub run_credential
 {
-- 
2.17.0

Re: [PATCH 1/2] git-credential-netrc: adapt to test framework for git

[Luis Marsano]

Junio C Hamano <gitster@pobox.com> wrote:

> Luis Marsano <luis.marsano@gmail.com> writes:
> 
> > until this change, git-credential-netrc did not test in a repository
> > this change reuses the main test framework, which provides such tests
> > specific changes
> 
> Sorry, but I cannot quite parse what the above is trying to say.
> 
[⋮]
> 
> Will queue, but may need to make the log message
> readable/understandable.
> 
> Thanks.

Thank you, the rewrite https://public-inbox.org/git/20180512091728.4931-2-luis.marsano@gmail.com/ should be more intelligible.
I welcome further criticisms if any.

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

[Luis Marsano]

Junio C Hamano <gitster@pobox.com> wrote:

> Luis Marsano <luis.marsano@gmail.com> 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.marsano@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 <placeholder>, 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 <placeholder> 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.marsano@gmail.com/ should reflect all corrections.
Thank you for the feedback, and please let me know of further issues.