* [PATCH] add -i: ignore terminal escape sequences
@ 2011-05-17 15:19 trast
2011-05-18 3:52 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: trast @ 2011-05-17 15:19 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King, Thomas Rast
From: Thomas Rast <trast@student.ethz.ch>
On the author's terminal, the up-arrow input sequence is ^[[A, and
thus fat-fingering an up-arrow into 'git checkout -p' is quite
dangerous: git-add--interactive.perl will ignore the ^[ and [
characters and happily treat A as "discard everything".
As a band-aid fix, use Term::Cap to get all terminal capabilities.
Then use the heuristic that any capability value that starts with ^[
(i.e., \e in perl) must be a key input sequence. Finally, given an
input that starts with ^[, read more characters until we have read a
full escape sequence, then return that to the caller. We use a
timeout of 0.5 seconds on the subsequent reads to avoid getting stuck
if the user actually input a lone ^[.
Since none of the currently recognized keys start with ^[, the net
result is that the sequence as a whole will be ignored and the help
displayed.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
I nearly managed to lose a bunch of uncommitted work today, but could
salvage most of it from the pieces of diffs in the terminal
scrollback. Sigh.
Future work might include mapping such sequences to the keys they
represent, so that (shift) up-arrow can be k (K) and (shift)
down-arrow j (J), or some such.
Oh yeah, PS: I'm alive ;-)
git-add--interactive.perl | 19 +++++++++++++++++++
1 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 4f08fe7..8f0839d 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -45,6 +45,9 @@
my $normal_color = $repo->get_color("", "reset");
my $use_readkey = 0;
+my $use_termcap = 0;
+my %term_escapes;
+
sub ReadMode;
sub ReadKey;
if ($repo->config_bool("interactive.singlekey")) {
@@ -53,6 +56,14 @@
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;
+ };
}
sub colored {
@@ -1067,6 +1078,14 @@ sub prompt_single_character {
ReadMode 'cbreak';
my $key = ReadKey 0;
ReadMode 'restore';
+ if ($use_termcap and $key eq "\e") {
+ while (!defined $term_escapes{$key}) {
+ my $next = ReadKey 0.5;
+ last if (!defined $next);
+ $key .= $next;
+ }
+ $key =~ s/\e/^[/;
+ }
print "$key" if defined $key;
print "\n";
return $key;
--
1.7.5.1.520.g98107.dirty
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] add -i: ignore terminal escape sequences
2011-05-17 15:19 [PATCH] add -i: ignore terminal escape sequences trast
@ 2011-05-18 3:52 ` Junio C Hamano
2011-05-18 4:37 ` Sverre Rabbelier
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2011-05-18 3:52 UTC (permalink / raw)
To: trast; +Cc: git, Jeff King
<trast@student.ethz.ch> writes:
> I nearly managed to lose a bunch of uncommitted work today, but could
> salvage most of it from the pieces of diffs in the terminal
> scrollback. Sigh.
I think the take-home lesson is that confirmation offered in the default
mode is valuable. The "single-key" mode is another long rope that I would
not use myself, but the users can choose to hang themselves with.
Jokes aside, it may make sense to offer an extra confirmation for "a" and
possibly "s" in single-key mode. Unlike others, they are destructive when
the changes you are splitting from the working tree is large-ish.
> Oh yeah, PS: I'm alive ;-)
Good to hear.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] add -i: ignore terminal escape sequences
2011-05-18 3:52 ` Junio C Hamano
@ 2011-05-18 4:37 ` Sverre Rabbelier
2011-05-18 5:03 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Sverre Rabbelier @ 2011-05-18 4:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: trast, git, Jeff King
Heya,
On Tue, May 17, 2011 at 20:52, Junio C Hamano <gitster@pobox.com> wrote:
> Jokes aside, it may make sense to offer an extra confirmation for "a" and
> possibly "s" in single-key mode. Unlike others, they are destructive when
> the changes you are splitting from the working tree is large-ish.
I don't understand why/how 's' is destructive? Doesn't it just ask you
again for each of the split hunks?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] add -i: ignore terminal escape sequences
2011-05-18 4:37 ` Sverre Rabbelier
@ 2011-05-18 5:03 ` Junio C Hamano
2011-05-18 5:54 ` Jeff King
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2011-05-18 5:03 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: trast, git, Jeff King
Sverre Rabbelier <srabbelier@gmail.com> writes:
> On Tue, May 17, 2011 at 20:52, Junio C Hamano <gitster@pobox.com> wrote:
>> Jokes aside, it may make sense to offer an extra confirmation for "a" and
>> possibly "s" in single-key mode. Unlike others, they are destructive when
>> the changes you are splitting from the working tree is large-ish.
>
> I don't understand why/how 's' is destructive? Doesn't it just ask you
> again for each of the split hunks?
I think "add -p" used to have an internal mechanism to merge the adjacent
split hunks back to a single hunk, so if we wanted to we could have given
users a way to recover from a mistaken "s"plit, but I don't think we kept
that code, so there is no way to properly "recover" from such a mistake.
Yes, it may be just the matter of two easy keystrokes, either yy or nn, to
recover from such a mistake, and that is why I said "possibly". It is
nevertheless destructive in the sense that you cannot recover without
quitting the current session and restarting.
Of course Thomas could have simply done "reset" and started from scratch,
so in that sense nothing is destructive, but we are not talking about the
kind of destructive operations you cannot possibly recover without typing
everything again.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] add -i: ignore terminal escape sequences
2011-05-18 5:03 ` Junio C Hamano
@ 2011-05-18 5:54 ` Jeff King
2011-05-18 7:33 ` Thomas Rast
0 siblings, 1 reply; 6+ messages in thread
From: Jeff King @ 2011-05-18 5:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, trast, git
On Tue, May 17, 2011 at 10:03:03PM -0700, Junio C Hamano wrote:
> I think "add -p" used to have an internal mechanism to merge the adjacent
> split hunks back to a single hunk, so if we wanted to we could have given
> users a way to recover from a mistaken "s"plit, but I don't think we kept
> that code, so there is no way to properly "recover" from such a mistake.
>
> Yes, it may be just the matter of two easy keystrokes, either yy or nn, to
> recover from such a mistake, and that is why I said "possibly". It is
> nevertheless destructive in the sense that you cannot recover without
> quitting the current session and restarting.
>
> Of course Thomas could have simply done "reset" and started from scratch,
> so in that sense nothing is destructive, but we are not talking about the
> kind of destructive operations you cannot possibly recover without typing
> everything again.
I'm not that concerned about these type of destructive operations, which
might waste a few seconds or a minute of your time. But Thomas' original
email indicated he was using "git checkout -p", which _is_ destructive
in a much bigger way, because a "y" overwrites worktree files which do
not otherwise have a backup (even "reset -p" leaves unreferenced blobs
that used to be in the index).
-Peff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] add -i: ignore terminal escape sequences
2011-05-18 5:54 ` Jeff King
@ 2011-05-18 7:33 ` Thomas Rast
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Rast @ 2011-05-18 7:33 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Sverre Rabbelier, git
Jeff King wrote:
> On Tue, May 17, 2011 at 10:03:03PM -0700, Junio C Hamano wrote:
>
> > Of course Thomas could have simply done "reset" and started from scratch,
> > so in that sense nothing is destructive, but we are not talking about the
> > kind of destructive operations you cannot possibly recover without typing
> > everything again.
>
> I'm not that concerned about these type of destructive operations, which
> might waste a few seconds or a minute of your time. But Thomas' original
> email indicated he was using "git checkout -p", which _is_ destructive
> in a much bigger way, because a "y" overwrites worktree files which do
> not otherwise have a backup (even "reset -p" leaves unreferenced blobs
> that used to be in the index).
Indeed. I wouldn't be complaining if I typed "a" or "y", but
'checkout -p' trashed all my changes to a file because it treated the
up-arrow as an "a". We should not accidentally "bind" things to extra
keys, much less dangerous ones. The fact that up-arrow is ^[[A is an
implementation detail.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-05-18 7:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-17 15:19 [PATCH] add -i: ignore terminal escape sequences trast
2011-05-18 3:52 ` Junio C Hamano
2011-05-18 4:37 ` Sverre Rabbelier
2011-05-18 5:03 ` Junio C Hamano
2011-05-18 5:54 ` Jeff King
2011-05-18 7:33 ` Thomas Rast
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).