diff mbox series

git-svn: drop FakeTerm hack

Message ID xmqqa5u888lz.fsf_-_@gitster.g (mailing list archive)
State Accepted
Commit aa4b83dd5e8e709f173027fc0fbbedbaf7c43d12
Headers show
Series git-svn: drop FakeTerm hack | expand

Commit Message

Junio C Hamano Aug. 30, 2023, 10:32 p.m. UTC 
Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
> ...
>> It could still benefit from cleaning up FakeTerm, since we lazily init
>> the object since 30d45f798d (git-svn: delay term initialization,
>> 2014-09-14). But I don't think there's a visible bug here with the new
>> version of Term::ReadLine::Gnu.
>
> True.  Let me drop the patch from the 'next down to master
> fast-track' candidate status.

We did the above but then everybody seems to have forgotten about
it.  Let's resurrect the topic.  Here is my attempt.

---- >8 ----
From: Wesley Schwengle <wesleys@opperschaap.net>
Subject: [PATCH] git-svn: drop FakeTerm hack

Drop the FakeTerm hack, just like dfd46bae (send-email: drop
FakeTerm hack, 2023-08-08) did, for exactly the same reason.

Signed-off-by: Wesley Schwengle <wesleys@opperschaap.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-svn.perl | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

Comments

Jeff King Aug. 31, 2023, 12:13 a.m. UTC | #1 
On Wed, Aug 30, 2023 at 03:32:08PM -0700, Junio C Hamano wrote:

> > True.  Let me drop the patch from the 'next down to master
> > fast-track' candidate status.
> 
> We did the above but then everybody seems to have forgotten about
> it.  Let's resurrect the topic.  Here is my attempt.
> 
> ---- >8 ----
> From: Wesley Schwengle <wesleys@opperschaap.net>
> Subject: [PATCH] git-svn: drop FakeTerm hack
> 
> Drop the FakeTerm hack, just like dfd46bae (send-email: drop
> FakeTerm hack, 2023-08-08) did, for exactly the same reason.

Yep, it looks good to me.

Optionally you could add this to the commit message:

  It has been obsolete in git-svn since 30d45f798d (git-svn: delay term
  initialization, 2014-09-14). Note that unlike send-email, we already
  make sure to load Term::ReadLine only once. So this is just a cleanup,
  and not fixing any bug.

-Peff
Junio C Hamano Aug. 31, 2023, 12:28 a.m. UTC | #2 
Jeff King <peff@peff.net> writes:

> Optionally you could add this to the commit message:
>
>   It has been obsolete in git-svn since 30d45f798d (git-svn: delay term
>   initialization, 2014-09-14). Note that unlike send-email, we already
>   make sure to load Term::ReadLine only once. So this is just a cleanup,
>   and not fixing any bug.

Thanks.  That reads extremely well.
diff mbox series

Patch

diff --git c/git-svn.perl w/git-svn.perl
index be987e316f..4e8878f035 100755
--- c/git-svn.perl
+++ w/git-svn.perl
@@ -297,28 +297,12 @@  sub _req_svn {
 		{} ],
 );
 
-package FakeTerm;
-sub new {
-	my ($class, $reason) = @_;
-	return bless \$reason, shift;
-}
-sub readline {
-	my $self = shift;
-	die "Cannot use readline on FakeTerm: $$self";
-}
-package main;
-
 my $term;
 sub term_init {
-	$term = eval {
-		require Term::ReadLine;
-		$ENV{"GIT_SVN_NOTTY"}
+	require Term::ReadLine;
+	$term = $ENV{"GIT_SVN_NOTTY"}
 			? new Term::ReadLine 'git-svn', \*STDIN, \*STDOUT
 			: new Term::ReadLine 'git-svn';
-	};
-	if ($@) {
-		$term = new FakeTerm "$@: going non-interactive";
-	}
 }
 
 my $cmd;