[PATCH 0/2] Some fixes for PTY with pseudo console support (2)

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] Some fixes for PTY with pseudo console support (2)

Takashi Yano
[PATCH 1/2] Cygwin: pty: Add a workaround for ^C handling.
- Pseudo console support introduced by commit
  169d65a5774acc76ce3f3feeedcbae7405aa9b57 sometimes cause random
  crash or freeze by pressing ^C while cygwin and non-cygwin
  processes are executed simultaneously in the same pty. This
  patch is a workaround for this issue.

[PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.
- Pseudo console support introduced by commit
  169d65a5774acc76ce3f3feeedcbae7405aa9b57 shows garbage ^[[H^[[J in
  some of emacs screens. These screens do not handle ANSI escape
  sequences. Therefore, clear screen is disabled on these screens.

Takashi Yano (2):
  Cygwin: pty: Add a workaround for ^C handling.
  Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

 winsup/cygwin/fhandler_tty.cc | 31 ++++++++++++++++++++++++-------
 winsup/cygwin/spawn.cc        |  6 ++++++
 winsup/cygwin/tty.cc          |  1 +
 winsup/cygwin/tty.h           |  1 +
 4 files changed, 32 insertions(+), 7 deletions(-)

--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Cygwin: pty: Add a workaround for ^C handling.

Takashi Yano
- Pseudo console support introduced by commit
  169d65a5774acc76ce3f3feeedcbae7405aa9b57 sometimes cause random
  crash or freeze by pressing ^C while cygwin and non-cygwin
  processes are executed simultaneously in the same pty. This
  patch is a workaround for this issue.
---
 winsup/cygwin/fhandler_tty.cc | 5 +++++
 winsup/cygwin/spawn.cc        | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 240ee017c..283558985 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -2846,6 +2846,9 @@ fhandler_pty_slave::fixup_after_fork (HANDLE parent)
   // fork_fixup (parent, inuse, "inuse");
   // fhandler_pty_common::fixup_after_fork (parent);
   report_tty_counts (this, "inherited", "");
+  if (getPseudoConsole ())
+    myself->ctty = 0; /* Avoid setting init_console_handler() in fork.cc.
+ This is a workaround for ^C handling problem. */
 }
 
 void
@@ -2882,6 +2885,8 @@ fhandler_pty_slave::fixup_after_exec ()
   FreeConsole ();
  }
     }
+  if (getPseudoConsole () && myself->ctty == 0)
+    myself->ctty = get_ttyp ()->ntty; /* Restore ctty */
 
 #if USE_API_HOOK
   /* Hook Console API */
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 4bb28c47b..22dafbce3 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -634,6 +634,12 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
  }
       if (ptys)
  ptys->fixup_after_attach (!iscygwin ());
+      if (attach_to_pcon && !iscygwin ())
+ {
+  myself->ctty = 0; /* Random freezes caused by ^C can be avoided
+       with this. */
+  init_console_handler (true);
+ }
 
     loop:
       /* When ruid != euid we create the new process under the current original
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Takashi Yano
In reply to this post by Takashi Yano
- Pseudo console support introduced by commit
  169d65a5774acc76ce3f3feeedcbae7405aa9b57 shows garbage ^[[H^[[J in
  some of emacs screens. These screens do not handle ANSI escape
  sequences. Therefore, clear screen is disabled on these screens.
---
 winsup/cygwin/fhandler_tty.cc | 26 +++++++++++++++++++-------
 winsup/cygwin/tty.cc          |  1 +
 winsup/cygwin/tty.h           |  1 +
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 283558985..a74c3eecf 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -972,6 +972,19 @@ skip_console_setting:
 void
 fhandler_pty_slave::reset_switch_to_pcon (void)
 {
+  if (get_ttyp ()->need_clear_screen)
+    {
+      const char *term = getenv ("TERM");
+      if (term && strcmp (term, "dumb") && !strstr (term, "emacs"))
+ {
+  /* FIXME: Clearing sequence may not be "^[[H^[[J"
+     depending on the terminal type. */
+  DWORD n;
+  WriteFile (get_output_handle_cyg (), "\033[H\033[J", 6, &n, NULL);
+ }
+      get_ttyp ()->need_clear_screen = false;
+    }
+
   if (ALWAYS_USE_PCON)
     return;
   if (isHybrid)
@@ -2808,14 +2821,13 @@ fhandler_pty_slave::fixup_after_attach (bool native_maybe)
       /* Clear screen to synchronize pseudo console screen buffer
  with real terminal. This is necessary because pseudo
  console screen buffer is empty at start. */
-      /* FIXME: Clearing sequence may not be "^[[H^[[J"
- depending on the terminal type. */
-      DWORD n;
-      if (get_ttyp ()->num_pcon_attached_slaves == 0
-  && !ALWAYS_USE_PCON)
+      const char *term = getenv ("TERM");
+      if (get_ttyp ()->num_pcon_attached_slaves == 0 &&
+  term && strcmp (term, "dumb") &&
+  term && !strstr (term, "emacs") &&
+  !ALWAYS_USE_PCON)
  /* Assume this is the first process using this pty slave. */
- WriteFile (get_output_handle_cyg (),
-   "\033[H\033[J", 6, &n, NULL);
+ get_ttyp ()->need_clear_screen = true;
 
       get_ttyp ()->num_pcon_attached_slaves ++;
     }
diff --git a/winsup/cygwin/tty.cc b/winsup/cygwin/tty.cc
index 9244267c0..c94aee3ba 100644
--- a/winsup/cygwin/tty.cc
+++ b/winsup/cygwin/tty.cc
@@ -243,6 +243,7 @@ tty::init ()
   pcon_pid = 0;
   num_pcon_attached_slaves = 0;
   TermCodePage = 20127; /* ASCII */
+  need_clear_screen = false;
 }
 
 HANDLE
diff --git a/winsup/cygwin/tty.h b/winsup/cygwin/tty.h
index d59b2027d..c2b0490d0 100644
--- a/winsup/cygwin/tty.h
+++ b/winsup/cygwin/tty.h
@@ -104,6 +104,7 @@ private:
   pid_t pcon_pid;
   int num_pcon_attached_slaves;
   UINT TermCodePage;
+  bool need_clear_screen;
 
 public:
   HANDLE from_master () const { return _from_master; }
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Brian Inglis
On 2019-09-03 19:46, Takashi Yano wrote:
> - Pseudo console support introduced by commit
>   169d65a5774acc76ce3f3feeedcbae7405aa9b57 shows garbage ^[[H^[[J in
>   some of emacs screens. These screens do not handle ANSI escape
>   sequences. Therefore, clear screen is disabled on these screens.

Dealing with escape sequences is way out of the scope of any pty driver.
It is up to the terminal emulator or applications running in the terminal to
handle terminal characteristics appropriately.

The pty driver should not touch *ANY* escape sequences coming from the system,
nor should it generate any to it, as TERM may not be set at the time, or
appropriately or usefully in some shells e.g. cmd or powershell.

Most folks probably use mintty or cmd as their Cygwin terminal, but some use
other terminals, like various flavours of xterm and rxvt, with ssh sessions in
and out, so they could be on Linux consoles or proprietary AIX/HP-UX/Sun
terminals, and operate properly as long as they have a good terminfo definition.

I see this issue as similar to the Windows text file handling changes required
when coreutils/textutils went POSIX and removed '\r\n' crlf handling to give the
same results as on Unix systems.
To handle terminal characteristics properly would require terminfo support in
the pty driver: I doubt anyone wants that, so the best approach is to do
nothing, and let the terminal or application handle it: they are more likely to
have the configuration options or hooks to do so easily.

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Takashi Yano
Hi Brian,

On Tue, 3 Sep 2019 20:47:14 -0600
Brian Inglis wrote:

> On 2019-09-03 19:46, Takashi Yano wrote:
> > - Pseudo console support introduced by commit
> >   169d65a5774acc76ce3f3feeedcbae7405aa9b57 shows garbage ^[[H^[[J in
> >   some of emacs screens. These screens do not handle ANSI escape
> >   sequences. Therefore, clear screen is disabled on these screens.
>
> Dealing with escape sequences is way out of the scope of any pty driver.
> It is up to the terminal emulator or applications running in the terminal to
> handle terminal characteristics appropriately.
>
> The pty driver should not touch *ANY* escape sequences coming from the system,
> nor should it generate any to it, as TERM may not be set at the time, or
> appropriately or usefully in some shells e.g. cmd or powershell.
>
> Most folks probably use mintty or cmd as their Cygwin terminal, but some use
> other terminals, like various flavours of xterm and rxvt, with ssh sessions in
> and out, so they could be on Linux consoles or proprietary AIX/HP-UX/Sun
> terminals, and operate properly as long as they have a good terminfo definition.
>
> I see this issue as similar to the Windows text file handling changes required
> when coreutils/textutils went POSIX and removed '\r\n' crlf handling to give the
> same results as on Unix systems.
> To handle terminal characteristics properly would require terminfo support in
> the pty driver: I doubt anyone wants that, so the best approach is to do
> nothing, and let the terminal or application handle it: they are more likely to
> have the configuration options or hooks to do so easily.
You are definitely right. However, the essence of the problem is that
the pseudo console itself outputs a lot of ANSI escape sequences
even if client program output only ASCII characters.

Attached is the raw output from pseudo console when the screen shows
the simple text below.

---- from here ----
[yano@Express5800-S70 ~]$ cmd
Microsoft Windows [Version 10.0.18362.329]
(c) 2019 Microsoft Corporation. All rights reserved.

C:\cygwin\home\yano>exit
[yano@Express5800-S70 ~]$ exit
exit
---- to here ----

You will noticed that the screen is cleared if you execute
cat pcon-output.log
in a terminal which support ANSI escape sequences.

So clearing the screen when creating pseudo console is the result of
compromise to synchronize real screen and screen buffer of pseudo
console.

--
Takashi Yano <[hidden email]>

pcon-output.log (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Takashi Yano
On Wed, 4 Sep 2019 12:34:31 +0900
Takashi Yano wrote:

> Attached is the raw output from pseudo console when the screen shows
> the simple text below.
>
> ---- from here ----
> [yano@Express5800-S70 ~]$ cmd
> Microsoft Windows [Version 10.0.18362.329]
> (c) 2019 Microsoft Corporation. All rights reserved.
>
> C:\cygwin\home\yano>exit
> [yano@Express5800-S70 ~]$ exit
> exit
> ---- to here ----
>
> You will noticed that the screen is cleared if you execute
> cat pcon-output.log
> in a terminal which support ANSI escape sequences.

This pcon-output.log was recorded in screen of 80x24 size.
Please try above in 80x24 terminal.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Cygwin: pty: Add a workaround for ^C handling.

Corinna Vinschen-2
In reply to this post by Takashi Yano
On Sep  4 10:46, Takashi Yano wrote:
> - Pseudo console support introduced by commit
>   169d65a5774acc76ce3f3feeedcbae7405aa9b57 sometimes cause random
>   crash or freeze by pressing ^C while cygwin and non-cygwin
>   processes are executed simultaneously in the same pty. This
>   patch is a workaround for this issue.

If this workaround works, what about making it the standard behaviour,
rather than pseudo-console only?  Would there be a downside?


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Corinna Vinschen-2
In reply to this post by Takashi Yano
On Sep  4 10:46, Takashi Yano wrote:

> - Pseudo console support introduced by commit
>   169d65a5774acc76ce3f3feeedcbae7405aa9b57 shows garbage ^[[H^[[J in
>   some of emacs screens. These screens do not handle ANSI escape
>   sequences. Therefore, clear screen is disabled on these screens.
> ---
>  winsup/cygwin/fhandler_tty.cc | 26 +++++++++++++++++++-------
>  winsup/cygwin/tty.cc          |  1 +
>  winsup/cygwin/tty.h           |  1 +
>  3 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
> index 283558985..a74c3eecf 100644
> --- a/winsup/cygwin/fhandler_tty.cc
> +++ b/winsup/cygwin/fhandler_tty.cc
> @@ -972,6 +972,19 @@ skip_console_setting:
>  void
>  fhandler_pty_slave::reset_switch_to_pcon (void)
>  {
> +  if (get_ttyp ()->need_clear_screen)
> +    {
> +      const char *term = getenv ("TERM");
> +      if (term && strcmp (term, "dumb") && !strstr (term, "emacs"))
Why do you check the TERMs again here?  After all, need_clear_screen
is only true if one of these terms are used.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Takashi Yano
On Wed, 4 Sep 2019 12:47:38 +0200
Corinna Vinschen wrote:
> Why do you check the TERMs again here?  After all, need_clear_screen
> is only true if one of these terms are used.

Because, emacs seems to set environment TERM after fixup_after_exec()
is called. At the first check, TERM has the value of the terminal
in which emacs is executed. The first check is just in case.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Cygwin: pty: Add a workaround for ^C handling.

Takashi Yano
In reply to this post by Corinna Vinschen-2
On Wed, 4 Sep 2019 12:42:22 +0200
Corinna Vinschen wrote:
> If this workaround works, what about making it the standard behaviour,
> rather than pseudo-console only?  Would there be a downside?

I am not sure why, but console does not have this issue.
However, I do not notice any downside.

If making it standard, the patch will be very simple as follows.


diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index a3a7e7505..0a929dffd 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -213,7 +213,6 @@ frok::child (volatile char * volatile here)
      - terminate the current fork call even if the child is initialized. */
   sync_with_parent ("performed fork fixups and dynamic dll loading", true);

-  init_console_handler (myself->ctty > 0);
   ForceCloseHandle1 (fork_info->forker_finished, forker_finished);

   pthread::atforkchild ();
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 4bb28c47b..15cba3610 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -635,6 +635,12 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
       if (ptys)
        ptys->fixup_after_attach (!iscygwin ());

+      if (!iscygwin ())
+       {
+         init_console_handler (myself->ctty > 0);
+         myself->ctty = 0;
+       }
+
     loop:
       /* When ruid != euid we create the new process under the current original
         account and impersonate in child, this way maintaining the different

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] Cygwin: pty: Add a workaround for ^C handling.

Corinna Vinschen-2
On Sep  4 22:30, Takashi Yano wrote:

> On Wed, 4 Sep 2019 12:42:22 +0200
> Corinna Vinschen wrote:
> > If this workaround works, what about making it the standard behaviour,
> > rather than pseudo-console only?  Would there be a downside?
>
> I am not sure why, but console does not have this issue.
> However, I do not notice any downside.
>
> If making it standard, the patch will be very simple as follows.
>
>
> diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
> index a3a7e7505..0a929dffd 100644
> --- a/winsup/cygwin/fork.cc
> +++ b/winsup/cygwin/fork.cc
> @@ -213,7 +213,6 @@ frok::child (volatile char * volatile here)
>       - terminate the current fork call even if the child is initialized. */
>    sync_with_parent ("performed fork fixups and dynamic dll loading", true);
>
> -  init_console_handler (myself->ctty > 0);
>    ForceCloseHandle1 (fork_info->forker_finished, forker_finished);
>
>    pthread::atforkchild ();
> diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
> index 4bb28c47b..15cba3610 100644
> --- a/winsup/cygwin/spawn.cc
> +++ b/winsup/cygwin/spawn.cc
> @@ -635,6 +635,12 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
>        if (ptys)
>         ptys->fixup_after_attach (!iscygwin ());
>
> +      if (!iscygwin ())
> +       {
> +         init_console_handler (myself->ctty > 0);
> +         myself->ctty = 0;
> +       }
> +
>      loop:
>        /* When ruid != euid we create the new process under the current original
>          account and impersonate in child, this way maintaining the different
Sounds good to me.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Corinna Vinschen-2
In reply to this post by Takashi Yano
On Sep  4 21:49, Takashi Yano wrote:
> On Wed, 4 Sep 2019 12:47:38 +0200
> Corinna Vinschen wrote:
> > Why do you check the TERMs again here?  After all, need_clear_screen
> > is only true if one of these terms are used.
>
> Because, emacs seems to set environment TERM after fixup_after_exec()
> is called. At the first check, TERM has the value of the terminal
> in which emacs is executed. The first check is just in case.

I still don't get it.

The code in fixup_after_attach() is the only code snippet setting
need_clear_screen = true.  And that code also requires term != "dump" &&
term == "*emacs*" to set need_clear_screen.

The code in reset_switch_to_pcon() requires that the need_clear_screen
flag is true regardless of checking TERM.  So this code depends on the
successful TERM check from fixup_after_attach anyway.

What am I missing?

While at it, in fixup_after_attach():

+             if (get_ttyp ()->num_pcon_attached_slaves == 0 &&
+                 term && strcmp (term, "dumb") &&
+            term && !strstr (term, "emacs") &&
+                 !ALWAYS_USE_PCON)

You're checking term for != NULL twice.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Takashi Yano
On Wed, 4 Sep 2019 15:55:03 +0200
Corinna Vinschen wrote:
> The code in fixup_after_attach() is the only code snippet setting
> need_clear_screen = true.  And that code also requires term != "dump" &&
> term == "*emacs*" to set need_clear_screen.

term != "*emacs*"

> The code in reset_switch_to_pcon() requires that the need_clear_screen
> flag is true regardless of checking TERM.  So this code depends on the
> successful TERM check from fixup_after_attach anyway.
>
> What am I missing?

Two checking results may not be the same. Indeed, emacs changes
TERM between two checks.

fixup_after_attach() is called from fixup_after_exec(),
which is called before executing the program code.
reset_switch_to_pcon () is mainly called from PTY slave I/O functions.
This is usually from the program code.

The behaviour of the patch is as follows.

First check : True  True  False False
Second check: True  False True  False
Clear screen: Yes   No    No   No

# True: neither dumb nor emacs*
#  False: either dumb or emacs*


> +             if (get_ttyp ()->num_pcon_attached_slaves == 0 &&
> +                 term && strcmp (term, "dumb") &&
> +            term && !strstr (term, "emacs") &&
> +                 !ALWAYS_USE_PCON)
>
> You're checking term for != NULL twice.

Oh my!

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Ken Brown-6
On 9/4/2019 10:42 AM, Takashi Yano wrote:

> On Wed, 4 Sep 2019 15:55:03 +0200
> Corinna Vinschen wrote:
>> The code in fixup_after_attach() is the only code snippet setting
>> need_clear_screen = true.  And that code also requires term != "dump" &&
>> term == "*emacs*" to set need_clear_screen.
>
> term != "*emacs*"
>
>> The code in reset_switch_to_pcon() requires that the need_clear_screen
>> flag is true regardless of checking TERM.  So this code depends on the
>> successful TERM check from fixup_after_attach anyway.
>>
>> What am I missing?
>
> Two checking results may not be the same. Indeed, emacs changes
> TERM between two checks.
>
> fixup_after_attach() is called from fixup_after_exec(),
> which is called before executing the program code.
> reset_switch_to_pcon () is mainly called from PTY slave I/O functions.
> This is usually from the program code.

But the first check (the one in fixup_after_attach()) could be dropped, couldn't it?

Ken
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Corinna Vinschen-2
On Sep  4 15:11, Ken Brown wrote:

> On 9/4/2019 10:42 AM, Takashi Yano wrote:
> > On Wed, 4 Sep 2019 15:55:03 +0200
> > Corinna Vinschen wrote:
> >> The code in fixup_after_attach() is the only code snippet setting
> >> need_clear_screen = true.  And that code also requires term != "dump" &&
> >> term == "*emacs*" to set need_clear_screen.
> >
> > term != "*emacs*"
> >
> >> The code in reset_switch_to_pcon() requires that the need_clear_screen
> >> flag is true regardless of checking TERM.  So this code depends on the
> >> successful TERM check from fixup_after_attach anyway.
> >>
> >> What am I missing?
> >
> > Two checking results may not be the same. Indeed, emacs changes
> > TERM between two checks.
> >
> > fixup_after_attach() is called from fixup_after_exec(),
> > which is called before executing the program code.
> > reset_switch_to_pcon () is mainly called from PTY slave I/O functions.
> > This is usually from the program code.
>
> But the first check (the one in fixup_after_attach()) could be dropped, couldn't it?
IIUC the second test first checks for need_clear_screen but then the
TERM might have changed in the meantime which in turn requires to change
the behaviour again.  But yeah, this sound like the first patch is not
actually required at all.


Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Takashi Yano
On Wed, 4 Sep 2019 17:19:05 +0200
Corinna Vinschen wrote:
> > But the first check (the one in fixup_after_attach()) could be dropped, couldn't it?
>
> IIUC the second test first checks for need_clear_screen but then the
> TERM might have changed in the meantime which in turn requires to change
> the behaviour again.  But yeah, this sound like the first patch is not
> actually required at all.

I was convinced. I will revise the patch.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Brian Inglis
In reply to this post by Takashi Yano
On 2019-09-03 21:45, Takashi Yano wrote:

> On Wed, 4 Sep 2019 12:34:31 +0900
> Takashi Yano wrote:
>> Attached is the raw output from pseudo console when the screen shows
>> the simple text below.
>>
>> ---- from here ----
>> [yano@Express5800-S70 ~]$ cmd
>> Microsoft Windows [Version 10.0.18362.329]
>> (c) 2019 Microsoft Corporation. All rights reserved.
>>
>> C:\cygwin\home\yano>exit
>> [yano@Express5800-S70 ~]$ exit
>> exit
>> ---- to here ----
>>
>> You will noticed that the screen is cleared if you execute
>> cat pcon-output.log
>> in a terminal which support ANSI escape sequences.
>
> This pcon-output.log was recorded in screen of 80x24 size.
> Please try above in 80x24 terminal.

That output seems to be generated by a shell or program running in the pty.
If the recipient can not handle escape sequences, then the shell or program in
the pty should be configured by setting e.g. TERM=dumb when launching the
terminal, or in the shell environment.

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Takashi Yano
On Wed, 4 Sep 2019 11:22:42 -0600
Brian Inglis wrote:
> That output seems to be generated by a shell or program running in the pty.
> If the recipient can not handle escape sequences, then the shell or program in
> the pty should be configured by setting e.g. TERM=dumb when launching the
> terminal, or in the shell environment.

No. Output is almost same even if setting TERM=dumb.
See attached log.

Consider what should be the output of pseudo console if console
application like below is executed.

#include <windows.h>

int main()
{
    HANDLE h = GetStdHandle(STD_OUTPUT_HANDLE);
    COORD p = {20, 10};
    DWORD n;
    SetConsoleCursorPosition(h, p);
    SetConsoleTextAttribute(h, FOREGROUND_RED);
    WriteConsole(h, "R", 1, &n, 0);
    SetConsoleTextAttribute(h, FOREGROUND_GREEN);
    WriteConsole(h, "G", 1, &n, 0);
    SetConsoleTextAttribute(h, FOREGROUND_BLUE);
    WriteConsole(h, "B", 1, &n, 0);
    SetConsoleTextAttribute(h, FOREGROUND_RED|FOREGROUND_GREEN|FOREGROUND_BLUE);
    WriteConsole(h, "W", 1, &n, 0);
    WriteConsole(h, "\r\n", 2, &n, 0);
    return 0;
}

Setting cursor posision and text color cannot be realized
without ANSI escape sequences.

Indeed, the output of the pseudo console by the program above is:
^[[?25l^[[11;21H^[[?25h^[[?25l^[[31mR^[[32mG^[[34mB^[[mW^M
^[[?25h

--
Takashi Yano <[hidden email]>

pcon-output-dumb.log (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Brian Inglis
On 2019-09-04 19:13, Takashi Yano wrote:

> On Wed, 4 Sep 2019 11:22:42 -0600
> Brian Inglis wrote:
>> That output seems to be generated by a shell or program running in the pty.
>> If the recipient can not handle escape sequences, then the shell or program in
>> the pty should be configured by setting e.g. TERM=dumb when launching the
>> terminal, or in the shell environment.
>
> No. Output is almost same even if setting TERM=dumb.
> See attached log.
>
> Consider what should be the output of pseudo console if console
> application like below is executed.
>
> #include <windows.h>
>
> int main()
> {
>     HANDLE h = GetStdHandle(STD_OUTPUT_HANDLE);
>     COORD p = {20, 10};
>     DWORD n;
>     SetConsoleCursorPosition(h, p);
>     SetConsoleTextAttribute(h, FOREGROUND_RED);
>     WriteConsole(h, "R", 1, &n, 0);
>     SetConsoleTextAttribute(h, FOREGROUND_GREEN);
>     WriteConsole(h, "G", 1, &n, 0);
>     SetConsoleTextAttribute(h, FOREGROUND_BLUE);
>     WriteConsole(h, "B", 1, &n, 0);
>     SetConsoleTextAttribute(h, FOREGROUND_RED|FOREGROUND_GREEN|FOREGROUND_BLUE);
>     WriteConsole(h, "W", 1, &n, 0);
>     WriteConsole(h, "\r\n", 2, &n, 0);
>     return 0;
> }
>
> Setting cursor posision and text color cannot be realized
> without ANSI escape sequences.
>
> Indeed, the output of the pseudo console by the program above is:
> ^[[?25l^[[11;21H^[[?25h^[[?25l^[[31mR^[[32mG^[[34mB^[[mW^M
> ^[[?25h

So how do you tell the pseudo-console to generate only text not escape sequences
the recipient may not be prepared to deal with?

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] Cygwin: pty: Disable clear screen on new pty if TERM=dumb or emacs*.

Takashi Yano
On Wed, 4 Sep 2019 21:36:28 -0600
Brian Inglis wrote:
> So how do you tell the pseudo-console to generate only text not escape sequences
> the recipient may not be prepared to deal with?

Unfortunately, no idea.

--
Takashi Yano <[hidden email]>