Repository: dankamongmen/notcurses (language: python)

Issue:
notcurses-demo leaves kitty in bad state
I've been investigating this. It seems that `notcurses-demo` leaves the *alternate screen* in a bad keyboard status when using the Kitty protocol. I think we're leaving via `rmcup` twice or something. New in 2.4.5.


Discussion / hints:

Fix patch:
diff --git a/src/lib/notcurses.c b/src/lib/notcurses.c
index 070b9f45e4..714246e55a 100644
--- a/src/lib/notcurses.c
+++ b/src/lib/notcurses.c
@@ -71,12 +71,12 @@ int reset_term_attributes(const tinfo* ti, fbuf* f){
 static int
 notcurses_stop_minimal(void* vnc){
   notcurses* nc = vnc;
+  int ret = 0;
+  ret |= drop_signals(nc);
   // collect output into the memstream buffer, and then dump it directly using
   // blocking_write(), to avoid problems with unreliable fflush().
   fbuf* f = &nc->rstate.f;
   fbuf_reset(f);
-  int ret = 0;
-  ret |= drop_signals(nc);
   // be sure to write the restoration sequences *prior* to running rmcup, as
   // they apply to the screen (alternate or otherwise) we're actually using.
   const char* esc;
@@ -100,17 +100,20 @@ notcurses_stop_minimal(void* vnc){
     if(nc->tcache.tpreserved){
       ret |= tcsetattr(nc->tcache.ttyfd, TCSAFLUSH, nc->tcache.tpreserved);
     }
+    // don't use use leave_alternate_screen() here; we need pop the keyboard
+    // whether we're in regular or altnerate screen, and we need it done
+    // before returning to the regular screen if we're in the alternate.
     if(nc->tcache.kbdlevel){
       if(tty_emit(KKEYBOARD_POP, nc->tcache.ttyfd)){
         ret = -1;
       }
     }
-    if((esc = get_escape(&nc->tcache, ESCAPE_RMCUP))){
-      if(sprite_clear_all(&nc->tcache, f)){ // send this to f
-        ret = -1;
-      }
-      if(tty_emit(esc, nc->tcache.ttyfd)){ // but this goes to ttyfd
-        ret = -1;
+    if(nc->tcache.in_alt_screen){
+      if((esc = get_escape(&nc->tcache, ESCAPE_RMCUP))){
+        if(tty_emit(esc, nc->tcache.ttyfd)){
+          ret = -1;
+        }
+        nc->tcache.in_alt_screen = 0;
       }
     }
   }
diff --git a/src/lib/termdesc.c b/src/lib/termdesc.c
index 37c80e128f..5460c3f2de 100644
--- a/src/lib/termdesc.c
+++ b/src/lib/termdesc.c
@@ -367,13 +367,10 @@ init_terminfo_esc(tinfo* ti, const char* name, escape_e idx,
 // request the cell geometry of the textual area
 #define GEOMCELL "\x1b[18t"
 
-#define DIRECTIVES CSI_BGQ \
+#define DIRECTIVES KBDQUERY \
+                   CSI_BGQ \
                    SUMQUERY \
                    "\x1b[?1;3;256S" /* try to set 256 cregs */ \
-                   KKEYBOARD_PUSH \
-                   XTMODKEYS \
-                   KBDSUPPORT \
-                   KBDQUERY \
                    KITTYQUERY \
                    CREGSXTSM \
                    GEOMXTSM \
@@ -381,11 +378,16 @@ init_terminfo_esc(tinfo* ti, const char* name, escape_e idx,
                    GEOMCELL \
                    PRIDEVATTR
 
+// written whenever we switch between standard and alternate screen, or upon
+// startup (that's an entry into a screen! presumably the standard one).
+#define KBDENTER KKEYBOARD_PUSH XTMODKEYS KBDSUPPORT
+#define KBDLEAVE KKEYBOARD_POP // FIXME undo XTMODKEYS!
+
 // enter the alternate screen (smcup). we could technically get this from
 // terminfo, but everyone who supports it supports it the same way, and we
 // need to send it before our other directives if we're going to use it.
-#define SMCUP "\x1b[?1049h"
-#define RMCUP "\x1b[?1049l"
+#define SMCUP "\x1b[?1049h" KBDENTER
+#define RMCUP KBDLEAVE "\x1b[?1049l"
 
 // we send an XTSMGRAPHICS to set up 256 color registers (the most we can
 // currently take advantage of; we need at least 64 to use sixel at all).
@@ -397,9 +399,9 @@ send_initial_queries(int fd, bool minimal, bool noaltscreen){
   const char *queries;
   if(noaltscreen){
     if(minimal){
-      queries = DSRCPR DIRECTIVES;
+      queries = KBDENTER DSRCPR DIRECTIVES;
     }else{
-      queries = DSRCPR IDQUERIES DIRECTIVES;
+      queries = KBDENTER DSRCPR IDQUERIES DIRECTIVES;
     }
   }else{
     if(minimal){
@@ -416,6 +418,40 @@ send_initial_queries(int fd, bool minimal, bool noaltscreen){
   return 0;
 }
 
+int enter_alternate_screen(FILE* fp, tinfo* ti, bool flush){
+  if(ti->in_alt_screen){
+    return 0;
+  }
+  const char* smcup = get_escape(ti, ESCAPE_SMCUP);
+  if(smcup == NULL){
+    logerror("alternate screen is unavailable");
+    return -1;
+  }
+  if(term_emit(smcup, fp, false) || term_emit(KBDENTER, fp, flush)){
+    return -1;
+  }
+  ti->in_alt_screen = true;
+  return 0;
+}
+
+int leave_alternate_screen(FILE* fp, tinfo* ti){
+  if(!ti->in_alt_screen){
+    return 0;
+  }
+  const char* rmcup = get_escape(ti, ESCAPE_RMCUP);
+  if(rmcup == NULL){
+    logerror("can't leave alternate screen");
+    return -1;
+  }
+  if(term_emit(KBDLEAVE, fp, false) ||
+     term_emit(rmcup, fp, false) ||
+     term_emit(KBDENTER, fp, true)){
+    return -1;
+  }
+  ti->in_alt_screen = false;
+  return 0;
+}
+
 // if we get a response to the standard cursor locator escape, we know this
 // terminal supports it, hah.
 static int
diff --git a/src/lib/termdesc.h b/src/lib/termdesc.h
index 18115c18d4..69ebca857e 100644
--- a/src/lib/termdesc.h
+++ b/src/lib/termdesc.h
@@ -321,39 +321,8 @@ term_emit(const char* seq, FILE* out, bool flush){
   return flush ? ncflush(out) : 0;
 }
 
-static inline int
-enter_alternate_screen(FILE* fp, tinfo* ti, bool flush){
-  if(ti->in_alt_screen){
-    return 0;
-  }
-  const char* smcup = get_escape(ti, ESCAPE_SMCUP);
-  if(smcup == NULL){
-    logerror("alternate screen is unavailable");
-    return -1;
-  }
-  if(term_emit(smcup, fp, flush)){
-    return -1;
-  }
-  ti->in_alt_screen = true;
-  return 0;
-}
-
-static inline int
-leave_alternate_screen(FILE* fp, tinfo* ti){
-  if(!ti->in_alt_screen){
-    return 0;
-  }
-  const char* rmcup = get_escape(ti, ESCAPE_RMCUP);
-  if(rmcup == NULL){
-    logerror("can't leave alternate screen");
-    return -1;
-  }
-  if(term_emit(rmcup, fp, true)){
-    return -1;
-  }
-  ti->in_alt_screen = false;
-  return 0;
-}
+int enter_alternate_screen(FILE* fp, tinfo* ti, bool flush);
+int leave_alternate_screen(FILE* fp, tinfo* ti);
 
 int cbreak_mode(tinfo* ti);
 


Test patch:
diff --git a/.github/workflows/windows_test.yml b/.github/workflows/windows_test.yml
index d560a7f6a6..4b76e9f9bf 100644
--- a/.github/workflows/windows_test.yml
+++ b/.github/workflows/windows_test.yml
@@ -55,7 +55,6 @@ jobs:
           cmake .. \
             -GNinja \
             -DCMAKE_BUILD_TYPE=Release \
-            -DUSE_DOCTEST=off \
             -DUSE_PANDOC=off \
             -DUSE_QRCODEGEN=on
 
@@ -67,7 +66,7 @@ jobs:
       - name: ctest
         run: |
           cd build
-          ctest --output-on-failure
+          #ctest --output-on-failure
 
       - name: make install
         run: |