johnnovak / illwill

A curses inspired simple cross-platform console library for Nim
Do What The F*ck You Want To Public License
398 stars 27 forks source link

Non-Fullscreen Does Not Restore Terminal State Properly on Windows #20

Closed Chris3606 closed 3 years ago

Chris3606 commented 3 years ago

Problem

illwillDeinit does not restore the terminal state properly for Windows. This can be outlined by the following modified simplekeycodes example, which throws an exception when 'A' is pressed and tries to de-initialize when exceptions are thrown:

# Simple non-full-screen example that prints out keycode enums of the keys
# pressed on the keyboard.
#
# Note that although echo, write and the terminal module should not be used
# when using illwill in fullscreen mode, in non-fullscreen mode it's okay to
# use them. In such case illwill is typically used for non-blocking keyboard
# input only, so it's best to import only the functions needed for that, as
# shown below. This way there will be no symbol name clashes when importing
# the terminal module (e.g. the foreground and background color enums).

import os, terminal
from illwill import illwillInit, illwillDeinit, getKey, Key

proc exitProc() {.noconv.} =
  illwillDeinit()
  quit(0)

proc main() =
  setControlCHook(exitProc)
  illwillInit(fullscreen = false)

  setForegroundColor(fgWhite)
  setStyle({styleDim})
  echo "-----------------------------------------------------------------"

  resetAttributes()
  setForegroundColor(fgYellow)
  echo "Press keys/key combinations on the keyboard to view their keycode"

  setForegroundColor(fgRed)
  echo "Press Ctrl-C to quit"

  setForegroundColor(fgWhite)
  setStyle({styleDim})
  echo "-----------------------------------------------------------------"

  setForegroundColor(fgWhite)
  setStyle({styleDim})

  resetAttributes()

  while true:
    var key = getKey()
    if key != Key.None:
      echo key
      if key == Key.A:
        raise newException(Exception, "Crash!")
    else:
      sleep(20)

try:
  main()
finally:
  illwillDeinit()

The stack trace nim prints will be formatted completely incorrectly: image

Underlying Issue/Suggested Solution

The following is the current consoleInit function for Windows:

  proc consoleInit() =
    discard getConsoleMode(getStdHandle(STD_INPUT_HANDLE), gOldConsoleModeInput.addr)
    if gFullScreen:
      if getConsoleMode(getStdHandle(STD_OUTPUT_HANDLE), gOldConsoleMode.addr) != 0:
        var mode = gOldConsoleMode and (not ENABLE_WRAP_AT_EOL_OUTPUT)
        discard setConsoleMode(getStdHandle(STD_OUTPUT_HANDLE), mode)

In the case where gFullScreen is set to false, gOldConsoleMode is never assigned to. However, regardless, consoleDeinit tries to restore from it:

  proc consoleDeinit() =
    discard setConsoleMode(getStdHandle(STD_OUTPUT_HANDLE), gOldConsoleMode)

This clearly doesn't restore the proper state for stdout since it wasn't recorded. Fixing this should be a fairly simple matter of rearranging the control flow. I have a working copy that just needs some error handling added; I'll try to put a PR in for this