This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Console requested reports â Re: [ANNOUNCEMENT] TEST RELEASE: Cygwin 2.5.0-0.6


Hi Thomas,


Thanks for the patch.  I have a few comments, though.

On Mar 15 14:16, Thomas Wolff wrote:
> On 13.03.2016, Corinna Vinschen wrote:
> >- Make buffered console characters visible to select().
> >   Addresses: https://cygwin.com/ml/cygwin/2014-12/msg00118.html
> Triggered by this change, I thought I'd revisit an old problem
> (https://cygwin.com/ml/cygwin-patches/2012-q3/msg00019.html),
> and in fact â requested console reports now work!
> This makes the following ESC sequences work:
> ESC[c sends primary device attributes
> ESC[>c sends secondary device attributes
> ESC[6n sends cursor position report
> 
> Changelog (old format):

Just drop this line from the comment, please.  If you send the mail
via git format-patch/git send-email I can simply apply it with git am
including the entire text in the git log.

> 2016-03-15  Thomas Wolff  <towo@towo.net>
> 
>     * fhandler.h (class dev_console): Add console read-ahead buffer.
>     (class fhandler_console): Add peek function for it (for select).
>     * fhandler_console.cc (fhandler_console::setup): Init buffer.
>     (fhandler_console::read): Check console read-aheader buffer.
>     (fhandler_console::char_command): Put responses to terminal
>     requests (device status and cursor position reports) into
>     common console buffer (shared between CONOUT/CONIN)
>     instead of fhandler buffer (separated).
>     * select.cc (peek_console): Check console read-ahead buffer.
> 
> Thomas

> diff -rup winsup/cygwin/orig/fhandler.h winsup/cygwin/fhandler.h
> --- winsup/cygwin/orig/fhandler.h	2016-03-10 17:30:40.000000000 +0000
> +++ winsup/cygwin/fhandler.h	2016-03-14 13:08:14.545958400 +0000
> @@ -1352,6 +1352,8 @@ class dev_console
>    bool ext_mouse_mode15;
>    bool use_focus;
>    bool raw_win32_keyboard_mode;
> +  char cons_rabuf[40];

Why 40?  Where does this number come from?  Do we have a define for this
which makes sense?  Shouldn't we introduce one if it doesn't exist?

> +  char * cons_rapoi;
          ^^^
          Drop the space, please.
>  
>    inline UINT get_console_cp ();
>    DWORD con_to_str (char *d, int dlen, WCHAR w);
> @@ -1449,6 +1451,7 @@ private:
>    int init (HANDLE, DWORD, mode_t);
>    bool mouse_aware (MOUSE_EVENT_RECORD& mouse_event);
>    bool focus_aware () {return shared_console_info->con.use_focus;}
> +  bool get_cons_readahead_valid () { return shared_console_info->con.cons_rapoi != 0; }

Can you please reformat this to fit into 80 columns?

Also, s/0/NULL/

>    select_record *select_read (select_stuff *);
>    select_record *select_write (select_stuff *);
> diff -rup winsup/cygwin/orig/fhandler_console.cc winsup/cygwin/fhandler_console.cc
> --- winsup/cygwin/orig/fhandler_console.cc	2016-01-12 14:39:52.000000000 +0000
> +++ winsup/cygwin/fhandler_console.cc	2016-03-15 13:12:29.273612200 +0000
> @@ -196,6 +196,7 @@ fhandler_console::setup ()
>  	  con.meta_mask |= RIGHT_ALT_PRESSED;
>  	con.set_default_attr ();
>  	con.backspace_keycode = CERASE;
> +	con.cons_rapoi = 0;

NULL, please.

>  	shared_console_info->tty_min_state.is_console = true;
>        }
>  }
> @@ -310,6 +311,14 @@ fhandler_console::read (void *pv, size_t
>    int ch;
>    set_input_state ();
>  
> +  /* Check console read-ahead buffer filled from terminal requests */
> +  if (con.cons_rapoi && * con.cons_rapoi)
                           ^^^
                           Drop the space, please.

> +    {
> +      * buf = * con.cons_rapoi ++;
         ^^^     ^^^
         Drop the spaces, please.

> +      buflen = 1;
> +      return;
> +    }
> +
>    int copied_chars = get_readahead_into_buffer (buf, buflen);
>  
>    if (copied_chars)
> @@ -1899,8 +1908,12 @@ fhandler_console::char_command (char c)
>  	strcpy (buf, "\033[?6c");
>        /* The generated report needs to be injected for read-ahead into the
>  	 fhandler_console object associated with standard input.
> -	 The current call does not work. */
> -      puts_readahead (buf);
> +	 So puts_readahead does not work. */
> +      //puts_readahead (buf);

Just remove this line entirely.

> +      /* Use a common console read-ahead buffer instead. */
> +      con.cons_rapoi = 0;
> +      strcpy (con.cons_rabuf, buf);
> +      con.cons_rapoi = con.cons_rabuf;
>        break;
>      case 'n':
>        switch (con.args[0])
> @@ -1910,7 +1923,10 @@ fhandler_console::char_command (char c)
>  	  y -= con.b.srWindow.Top;
>  	  /* x -= con.b.srWindow.Left;		// not available yet */
>  	  __small_sprintf (buf, "\033[%d;%dR", y + 1, x + 1);
> -	  puts_readahead (buf);
> +	  //puts_readahead (buf);

Ditto.

> +	  con.cons_rapoi = 0;
> +	  strcpy (con.cons_rabuf, buf);
> +	  con.cons_rapoi = con.cons_rabuf;
>  	  break;
>        default:
>  	  goto bad_escape;
> diff -rup winsup/cygwin/orig/select.cc winsup/cygwin/select.cc
> --- winsup/cygwin/orig/select.cc	2016-02-18 13:10:46.000000000 +0000
> +++ winsup/cygwin/select.cc	2016-03-14 13:09:07.661269400 +0000
> @@ -845,6 +845,12 @@ peek_console (select_record *me, bool)
>    if (!me->read_selected)
>      return me->write_ready;
>  
> +  if (fh->get_cons_readahead_valid ())
> +    {
> +      select_printf ("cons_readahead");
> +      return me->read_ready = true;
> +    }
> +
>    if (fh->get_readahead_valid ())
>      {
>        select_printf ("readahead");

Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]