Merge branch 'johan/resume-after-editor'

Fixes #381.

Author: walles

cmd/moor/moor.go

@@ -745,13 +745,6 @@ func startPaging(pager *internal.Pager, screen twin.Screen, chromaStyle *chroma.
 		if !pager.DeInit {
 			pager.ReprintAfterExit()
 		}
-
-		if pager.AfterExit != nil {
-			err := pager.AfterExit()
-			if err != nil {
-				log.Error("Failed running AfterExit hook: ", err)
-			}
-		}
 	}()
 
 	pager.StartPaging(screen, chromaStyle, chromaFormatter)

internal/editor.go

@@ -69,6 +69,9 @@ func errUnlessExecutable(file string) error {
 	return fmt.Errorf("Not executable: %s", file)
 }
 
+// pickAnEditor returns the editor command to execute, where it came from
+// ("VISUAL", "EDITOR" or "fallback list"), and an error if no editor could
+// be found.
 func pickAnEditor() (string, string, error) {
 	// Get an editor setting from either VISUAL or EDITOR
 	editorEnv := "VISUAL"
@@ -168,7 +171,7 @@ func handleEditingRequest(p *Pager) {
 		}
 	}
 
-	p.AfterExit = func() error {
+	err = p.screen.PauseAndCall(func() error {
 		// NOTE: If you do any changes here, make sure they work with both "nano"
 		// and "code -w" (VSCode).
 		commandWithArgs := strings.Fields(editor)
@@ -197,6 +200,14 @@ func handleEditingRequest(p *Pager) {
 			log.Info("Editor exited successfully: ", commandWithArgs)
 		}
 		return err
+	})
+	if err != nil {
+		log.Warn("Failed to launch editor in paused session: ", err)
+		p.mode = &PagerModeInfo{
+			Pager: p,
+			Text:  "Failed to launch editor \"" + editor + "\": " + err.Error(),
+		}
+
+		return
 	}
-	p.Quit()
 }

internal/pager.go

@@ -132,8 +132,6 @@ type Pager struct {
 	//
 	// Ref: https://github.com/walles/moor/issues/175
 	bookmarks map[rune]scrollPosition
-
-	AfterExit func() error
 }
 
 type _PreHelpState struct {

twin/fake-screen.go

@@ -117,3 +117,9 @@ func (screen *FakeScreen) Events() chan Event {
 func (screen *FakeScreen) GetRow(row int) []StyledRune {
 	return withoutHiddenRunes(screen.cells[row])
 }
+
+func (screen *FakeScreen) PauseAndCall(run func() error) error {
+	// The fake screen doesn't have any special state to save and restore, just
+	// run it.
+	return run()
+}

twin/screen-suspend.go

@@ -7,8 +7,6 @@ import (
 	"os"
 	"os/signal"
 	"syscall"
-
-	"golang.org/x/term"
 )
 
 // Suspend and wait for SIGCONT, then resume. Basically ctrl-Z handling.
@@ -19,47 +17,16 @@ func (screen *UnixScreen) suspend() error {
 	signal.Notify(cont, syscall.SIGCONT)
 	defer signal.Stop(cont)
 
-	screen.leaveAlternateScreenSession()
-
-	err := screen.restoreTtyInTtyOut()
-	if err != nil {
-		return fmt.Errorf("failed to restore terminal state before suspend: %w", err)
-	}
-
-	// kill(0) = "Send signal to all processes in the current process group"
-	err = syscall.Kill(0, syscall.SIGTSTP)
-	if err != nil {
-		restoreRawErr := screen.restoreRawModeAfterResume()
-		if restoreRawErr != nil {
-			return fmt.Errorf("failed to suspend process group: %w; also failed to re-enter raw mode: %v", err, restoreRawErr)
+	return screen.PauseAndCall(func() error {
+		// kill(0) = "Send signal to all processes in the current process group"
+		err := syscall.Kill(0, syscall.SIGTSTP)
+		if err != nil {
+			return fmt.Errorf("failed to suspend process group: %w", err)
 		}
 
-		screen.enterAlternateScreenSession()
-		screen.onWindowResized()
-
-		return fmt.Errorf("failed to suspend process group: %w", err)
-	}
-
-	// Wait for SIGCONT signal to arrive
-	<-cont
-
-	err = screen.restoreRawModeAfterResume()
-	if err != nil {
-		return err
-	}
-
-	screen.enterAlternateScreenSession()
-	screen.onWindowResized()
-
-	return nil
-}
-
-func (screen *UnixScreen) restoreRawModeAfterResume() error {
-	terminalState, err := term.MakeRaw(int(screen.ttyIn.Fd()))
-	if err != nil {
-		return fmt.Errorf("failed to re-enter raw mode after suspend: %w", err)
-	}
+		// Wait for SIGCONT signal to arrive
+		<-cont
 
-	screen.oldTerminalState = terminalState
-	return nil
+		return nil
+	})
 }

twin/screen.go

@@ -72,6 +72,10 @@ type Screen interface {
 
 	// This channel is what your main loop should be checking.
 	Events() chan Event
+
+	// Pause the screen, run the given function, then resume the screen. Blocks
+	// until the function has completed and the screen has been resumed again.
+	PauseAndCall(run func() error) error
 }
 
 type interruptableReader interface {
@@ -1132,3 +1136,43 @@ func (screen *UnixScreen) showNLines(width int, height int, clearFirst bool) {
 	screen.writeLocked(builder.String())
 	screen.snapshotLastRenderedLocked()
 }
+
+// Pause the screen, run the given function, then resume the screen. Blocks
+// until the function has completed and the screen has been resumed again.
+//
+// Error returns mean that either pausing failed or the run function failed. If
+// resuming fails, this method will panic.
+func (screen *UnixScreen) PauseAndCall(run func() error) error {
+	screen.leaveAlternateScreenSession()
+
+	err := screen.restoreTtyInTtyOut()
+	if err != nil {
+		return fmt.Errorf("failed to restore terminal state before pause: %w", err)
+	}
+
+	runErr := run()
+
+	restoreRawErr := screen.restoreRawModeAfterResume()
+	if restoreRawErr != nil {
+		panic(fmt.Errorf("failed to resume screen after paused operation (%v): %w", runErr, restoreRawErr))
+	}
+
+	screen.enterAlternateScreenSession()
+	screen.onWindowResized()
+
+	if runErr != nil {
+		return runErr
+	}
+
+	return nil
+}
+
+func (screen *UnixScreen) restoreRawModeAfterResume() error {
+	terminalState, err := term.MakeRaw(int(screen.ttyIn.Fd()))
+	if err != nil {
+		return fmt.Errorf("failed to re-enter raw mode after suspend: %w", err)
+	}
+
+	screen.oldTerminalState = terminalState
+	return nil
+}