Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CursesDriver should emit the "Set cursor key to application mode" ANSI escape code when it exits #418

Closed
tig opened this issue May 13, 2020 · 6 comments · Fixed by #953
Closed
Assignees
Labels

Comments

@tig
Copy link
Collaborator

tig commented May 13, 2020

out-consolegridview currently has this clean-up code in it to fix PowerShell/ConsoleGuiTools#49.

     public void Dispose()
        {
            // By emitting this, we fix an issue where arrow keys don't work in the console
            // because .NET requires application mode to support Arrow key escape sequences
            // Esc[?1h - Set cursor key to application mode
            // See http://ascii-table.com/ansi-escape-sequences-vt-100.php
            Console.Write("\u001b[?1h");
        }

Driver's clean-up code should emit the \u001b[?1h ANSI escape code when it exits.

According to @TylerLeonhardt he put this here as a quick fix, but it should really be fixed in gui.cs. See: PowerShell/ConsoleGuiTools#99 (comment)

The issue is not reproduceable in ConEmu or Windows Terminal on Windows, so this appears to be specific to Linux and Mac. I believe this should be done in the CursesDriver's implementation of ConsoleDriver.End() which currently looks like this:

	public override void End () => Curses.endwin ();

Thus:

	public override void End () 
	{
		// See gui.cs Issue #418
		Console.Out.Write ("\x1b[?1h");
		Console.Out.Flush ();
		Curses.endwin();
	}

But I don't know enough about Curses to know if this actually makes sense.

@TylerLeonhardt
Copy link

If possible this should reset it only if that's what it was set to before launching a gui.cs app.

@tig tig changed the title Drivers should emit the "Set cursor key to application mode" ANSI escape code when it exits CursesDriver should emit the "Set cursor key to application mode" ANSI escape code when it exits May 15, 2020
@tig tig self-assigned this May 19, 2020
@tig tig added the bug label May 19, 2020
@tig tig closed this as completed Sep 28, 2020
@tig
Copy link
Collaborator Author

tig commented Oct 12, 2020

Reopening. See $945.

@tig tig reopened this Oct 12, 2020
@BDisp
Copy link
Collaborator

BDisp commented Oct 12, 2020

If possible this should reset it only if that's what it was set to before launching a gui.cs app.

The main problem is how to get it before application launch and assign at exit.

@BDisp
Copy link
Collaborator

BDisp commented Oct 12, 2020

public override void End ()
{
// See gui.cs Issue #418
Console.Out.Write ("\x1b[?1h");
Console.Out.Flush ();
Curses.endwin();
}

I've already tested that before with no success.

@BDisp
Copy link
Collaborator

BDisp commented Oct 13, 2020

@tig after all works but not alone. Before initscr must call

				Console.Out.Write ("\x1b[?1h");
				Console.Out.Flush ();

and before endwin must call

			Console.Out.Write ("\x1b[?1l");
			Console.Out.Flush ();

I think that the most correct is 1h on start and 1l at end like the mouse.

@migueldeicaza
Copy link
Collaborator

Mhm, I see what is going on here.

So PowerShell is expecting regular cursor key commands, while gui.cs uses Application Key commands.

The proper way of doing this would be to lookup the terminfo database and output the strings for rmkx and smkx.

You can use tigetstr to get the values of those strings.

@tig tig closed this as completed in #953 Apr 27, 2021
tig pushed a commit that referenced this issue Apr 27, 2021
* Fixes #931. Unix terminal hangs after exit.

* Changed escape sequence as suggested by @mklement0

* Changing as documented at http://ascii-table.com/ansi-escape-sequences-vt-100.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants