From 676b9e528260337a715d06be725ef1e8d0f5ee3d Mon Sep 17 00:00:00 2001
From: Michael Mikonos <127171689+mknos@users.noreply.github.com>
Date: Tue, 14 Nov 2023 21:56:38 +0800
Subject: [PATCH] mail: duplicated VISUAL code

* Introduce main::vipath() to avoid duplicated access of $ENV{VISUAL}
* Catch failure to spawn VISUAL editor by checking system() return value
* Delete "print Dumper" statements which were silently failing (maybe ancient debug code)
* When re-opening tmpfile in edit mode, file should be in read mode not write mode
* editor::edit() is invoked by the 'm' command to write a new message, then the special ~v command loads the buffer in vi
* editor::edit() input loop is terminated by a line with only a '.'
* Add File::Temp, as done in main::visual()
---
 bin/mail | 48 +++++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/bin/mail b/bin/mail
index 33955515..9be23072 100755
--- a/bin/mail
+++ b/bin/mail
@@ -510,6 +510,8 @@ package
 	editor; # hide from PAUSE
 @ISA=qw(PerlPowerTools::mailprog);
 
+use File::Temp;
+
 %editor::fields=(message => undef, mesgno => undef);
 sub new {
 	my $class=shift;
@@ -603,19 +605,17 @@ EDLOOP: {
 				print "read $arg $bytes bytes\n";
 			};
 			/v/ && do {
-				if (! $ENV{VISUAL}) {
-					$ENV{VISUAL}='/usr/bin/vi' if ( -x '/usr/bin/vi');
-					if (! $ENV{VISUAL}) {
-						warn "No VISUAL in environment\n";
-						last SWITCH;
-					}
-				}
-				open(F, '>', "/tmp/ppt_mail$$") || do { warn "Unable to open temp file"; last SWITCH; };
+				my $vipath = main::vipath() || last SWITCH;
+				my $tmp = File::Temp->new;
+				my $tmpfile = $tmp->filename;
 				@BODY=grep(s/$/\n/g, @BODY);
-				print F @BODY;
-				close(F);
-				system("$ENV{VISUAL} /tmp/ppt_mail$$");
-				open(F, '>', "/tmp/ppt_mail$$") || die "Unable to re-open temp file";
+				print { $tmp } @BODY;
+				my $rc = system($vipath, $tmpfile);
+				if ($rc != 0) {
+					warn "Failed to execute '$vipath': $!\n";
+					last SWITCH;
+				}
+				open(F, '<', $tmpfile) || die "Unable to re-open $tmpfile: $!";
 				@BODY=<F>;
 				chomp(@BODY);
 				close(F);
@@ -684,6 +684,13 @@ sub listing {
 	}
 	return $first;
 }
+sub vipath {
+	return $ENV{'VISUAL'} if (defined $ENV{'VISUAL'});
+	my $default = '/usr/bin/vi';
+	return $default if (-x $default);
+	warn "No VISUAL in environment\n";
+	return;
+}
 sub shell {
 	# How to get an interactive shell in Perl.  Hmmm...
 	system("/bin/sh");   # For now.  :-)
@@ -783,14 +790,7 @@ sub quit {
 sub visual {
 	my($list)=@_;
 
-	if (! $ENV{VISUAL}) {
-		$ENV{VISUAL}='/usr/bin/vi' if ( -x '/usr/bin/vi');
-		if (! $ENV{VISUAL}) {
-			warn "No VISUAL in environment\n";
-			return;
-		}
-	}
-
+	my $cmd = vipath() || return;
 	foreach my $msgno (@$list) {
 		$message=$box->messagex($msgno);
 		if (! defined $message) {
@@ -802,11 +802,13 @@ sub visual {
 		my $tmbox = mailbox->new('file' => $path);
 		$tmbox->stuff($message);
 		$tmbox->write;
-		system($ENV{'VISUAL'}, $path);
+		my $rc = system($cmd, $path);
+		if ($rc != 0) {
+			warn "Failed to execute '$cmd': $!\n";
+			return;
+		}
 		$tmbox2 = mailbox->new('file' => $path);  # Hope this isn't a leak
-		print Dumper $tmbox2;
 		$tmbox2->load;
-		print Dumper $tmbox2;
 		$box->replace($msgno, $tmbox2->messagex(1));
 	}
 }