Re: [PATCH] git-config: document interactive.singlekey requires Term::Readkey

Simon Ruderich [off-list ref] writes:

Most distributions don't require Term::Readkey as dependency,
leaving the user to wonder why the setting doesn't work.

Signed-off-by: Simon Ruderich <redacted>

Thanks, but is it true that interactive.singlekey "requries"
Term::ReadKey?

The relevant part of git-add--interactive reads like so:

if ($repo->config_bool("interactive.singlekey")) {
	eval {
		require Term::ReadKey;
		Term::ReadKey->import;
		$use_readkey = 1;
	};
	eval {
		require Term::Cap;
		my $termcap = Term::Cap->Tgetent;
		foreach (values %$termcap) {
			$term_escapes{$_} = 1 if /^\e/;
		}
		$use_termcap = 1;
	};
}

The implementation of prompt_single_character sub wants to use
ReadKey, but can still let the user interact with the program by
falling back to a cooked input when it is not available, so perhaps
a better fix might be something like this:

        if (!$use_readkey) {
        	print STDERR "missing Term::ReadKey, disabling interactive.singlekey\n";
        }

inside the above if() that prepares $use_readkey?

You also misspelled the package name it seems ;-)

Most distributions don't require Term::ReadKey as dependency, leaving
the user to wonder why the setting doesn't work.

Signed-off-by: Simon Ruderich <redacted>
---

On Mon, Mar 03, 2014 at 10:58:58AM -0800, Junio C Hamano wrote:

Thanks, but is it true that interactive.singlekey "requries"
Term::ReadKey?

Yes, it requires it. The code also works fine without
Term::ReadKey, but the feature "singlekey" requires this module.
I assumed a user enabling this option would also want to use the
feature, therefore "requires" is fine IMHO.

The implementation of prompt_single_character sub wants to use
ReadKey, but can still let the user interact with the program by
falling back to a cooked input when it is not available, so perhaps
a better fix might be something like this:

        if (!$use_readkey) {
        	print STDERR "missing Term::ReadKey, disabling interactive.singlekey\n";
        }

inside the above if() that prepares $use_readkey?

Good idea. Implemented in an additional patch.

I think the documentation should also be updated (this patch) to
make it clear to a reader of the man page, that an additional
module is required, without having him to try to use the option.

You also misspelled the package name it seems ;-)

Oops, sorry. Fixed in this reroll.

Regards
Simon

 Documentation/config.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f4d793..406a582 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt

@@ -1633,7 +1633,7 @@ interactive.singlekey::
 	linkgit:git-add[1], linkgit:git-checkout[1], linkgit:git-commit[1],
 	linkgit:git-reset[1], and linkgit:git-stash[1]. Note that this
 	setting is silently ignored if portable keystroke input
-	is not available.
+	is not available; requires the Perl module Term::ReadKey.
 
 log.abbrevCommit::
 	If true, makes linkgit:git-log[1], linkgit:git-show[1], and

-- 
1.9.0.11.g9a08b42

-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9

Suggested-by: Junio C Hamano <redacted>
Signed-off-by: Simon Ruderich <redacted>
---
 git-add--interactive.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 24bb1ab..d3bca12 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl

@@ -58,6 +58,9 @@ if ($repo->config_bool("interactive.singlekey")) {
 		Term::ReadKey->import;
 		$use_readkey = 1;
 	};
+	if (!$use_readkey) {
+		print STDERR "missing Term::ReadKey, disabling interactive.singlekey\n";
+	}
 	eval {
 		require Term::Cap;
 		my $termcap = Term::Cap->Tgetent;

-- 
1.9.0.11.g9a08b42

-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9