martanne / vis

A vi-like editor based on Plan 9's structural regular expressions
Other
4.19k stars 259 forks source link

Properly restore terminal after running fullscreen program #1105

Closed jorbakk closed 11 months ago

jorbakk commented 1 year ago

When running a fullscreen program (curses) with vis:pipe() the terminal is not restored correctly. There's no way to automatically detect whether a program has to be run in shell or fullscreen mode, so a parameter needs to be added to vispipe*() and also vis:pipe() in the Lua API. This is useful for e.g. running interactive programs like nnn as a file picker, see https://github.com/captaingroove/vis-nnn.

rnpnr commented 1 year ago

Also can't you just call vis:redraw() after your command has run and returned?

jorbakk commented 1 year ago

Unfortunately not, the terminal state needs to be saved before running the command. That's in ui_term_backend_save(), which for the curses backend is different for fullscreen and shell mode programs:

static void ui_curses_save(UiTerm *tui, bool fscr) {
    curs_set(1);
    if (fscr) {
        def_prog_mode();
        endwin();
    } else {
        reset_shell_mode();
    }
}
rnpnr commented 1 year ago

So now that I better understand this I think this fix is fine. But I think there should be a way of doing it without the need for an extra function parameter. I'm not super familiar with curses and the documentation is not that helpful. From what I see here I'm not even sure that calling reset_shell_mode() was correct to begin with.

jorbakk commented 1 year ago

I think using reset_shell_mode() here is non-standard but unique. Then curses mode is not completely left and the vis editor window is scrolled while the subprocess writes to stdout. This is how vis composes other processes with the UI without opening a separate window that has terminal emulation capabilities.

Maybe the following simple program illustrates the two alternatives. By default, it runs the subprocess in shell mode and when providing an arbitrary command line argument, it leaves curses properly before running the subprocess. You might also change the subprocess to either shell or curses commands.

#include <stdlib.h>
#include <ncurses.h>

int
main(int argc, char** argv)
{
    initscr();                  /// Start curses mode
    printw("Hello curses!\n");  /// Print Hello World
    refresh();                  /// Print it on to the real screen
    getch();                    /// Pause, to see what happened

    if (argc > 1) {
        def_prog_mode();        /// Save the tty modes
        endwin();               /// End curses mode temporarily
    } else {
        reset_shell_mode();
    }
    // system("/bin/sh");       /// Do whatever you like in cooked mode
    system("ls -l");
    // system("nnn");

    getch();                    /// Pause, to see what happened
    reset_prog_mode();          /// Return to the previous tty mode stored by def_prog_mode()
    refresh();                  /// Do refresh() to restore the Screen contents
    printw("Another string\n"); /// Back to curses, use the full capabilities of curses
    refresh();
    getch();                    /// Pause, to see what happened
    endwin();                   /// End curses mode
    return 0;
}
ninewise commented 1 year ago

This would be useful to have indeed. The API change seems minor.

rnpnr commented 11 months ago

I guess what I want to know is if its possible to achieve this without the extra parameter. Is there a reason why we can't just always call:

def_prog_mode();
endwin();

From some very brief testing I'm not really seeing an issue with this.

jorbakk commented 11 months ago

I guess what I want to know is if its possible to achieve this without the extra parameter. Is there a reason why we can't just always call:

def_prog_mode();
endwin();

From some very brief testing I'm not really seeing an issue with this.

The behavior is different. Using

def_prog_mode();
endwin();

prints the output of the subprocess to the terminal buffer while hiding the curses buffer of the parent process.

In contrast

reset_shell_mode();

scrolls the curses buffer of the parent process while printing the output of the subprocess. The latter is the default behaviour in vis while the former is desirable for subprocesses that also use curses.

rnpnr commented 11 months ago

The behavior is different.

I understood that much but I thought it didn't matter. But I realized I didn't try vis-open.

I'm still not the biggest fan but @ninewise is correct in saying that the API change is minor. Can you rebase this onto master? Also I think you missed a doxygen comment for vis_pipe_collect().

jorbakk commented 11 months ago

Can you rebase this onto master? Also I think you missed a doxygen comment for vis_pipe_collect().

Okay, added the comments and rebased.

rnpnr commented 11 months ago

Applied, thanks for answering all my questions and addressing the issues!