Thread: readline/libedit patch

Started: 2008-10-07 22:14:37
Last activity: 2008-10-10 02:36:08
Topics: SAC Developers
Brian Savage
readline/libedit patch
2008-10-07 22:14:37
Kuang

I was looking at your readline patch to handle the real readline
library when used with a script.
http://www.iris.washington.edu/pipermail/sac-dev/2008-September/
000109.html

It looks like when the tty is not being used (use_tty()) you avoid
the select_loop().
This seems fine with me. We also have very similar code in the zgtmsg
() and
the code you added looks a lot like the original zgpmsg() code. Can
we do some
consolidation here, possibly just within the zgpmsg() to begin with.
and include code
when with HAVE_LIBEDIT or HAVE_READLINE ( currently just READLINE) is
true.
To make the the code easier to handle in the future I would prefer
the libedit and
readline behave the same way if a tty is, or is not, encountered.

- Use the select_loop() while in a tty session.
- Avoid the select_loop() while no tty is available
- Avoid the select_loop() while no line handling is available

How about something like:

#ifdef HAVE_READLINE || HAVE_LIBEDIT
if( use_tty() ) {
select_loop()
} else {
#endif /* HAVE READLINE || HAVE_LIBEDIT */

getfline()

#ifdef HAVE_READLINE || HAVE_LIBEDIT
}
#endif /* HAVE READLINE || HAVE_LIBEDIT */

For reference:
zgtmsg() is only called from
src/exm/xpause.c
src/exm/xnews.c
src/cpf/cresp.c (Standard Error Recovery, not sure this is used)

Cheers
Brian





  • Kuang He
    Re: readline/libedit patch
    2008-10-08 04:23:29
    On Tue, Oct 7, 2008 at 3:14 PM, Brian Savage <savage<at>uri.edu> wrote:
    I was looking at your readline patch to handle the real readline library
    when used with a script.
    http://www.iris.washington.edu/pipermail/sac-dev/2008-September/000109.html

    It looks like when the tty is not being used (use_tty()) you avoid the
    select_loop().
    This seems fine with me. We also have very similar code in the zgtmsg() and
    the code you added looks a lot like the original zgpmsg() code.

    Dear Brian,

    I've been meaning to ask you about the similar codes in zgtmsg.c and
    zgpmsg.c. How come we need two sets of those functions in the first
    place?

    Can we do some
    consolidation here, possibly just within the zgpmsg() to begin with. and
    include code
    when with HAVE_LIBEDIT or HAVE_READLINE ( currently just READLINE) is true.

    Ideally, we could totally get rid of the file zgtmsg.c, couldn't we?

    To make the the code easier to handle in the future I would prefer the
    libedit and
    readline behave the same way if a tty is, or is not, encountered.

    - Use the select_loop() while in a tty session.
    - Avoid the select_loop() while no tty is available
    - Avoid the select_loop() while no line handling is available

    How about something like:

    #ifdef HAVE_READLINE || HAVE_LIBEDIT
    if( use_tty() ) {
    select_loop()
    } else {
    #endif /* HAVE READLINE || HAVE_LIBEDIT */

    getfline()

    #ifdef HAVE_READLINE || HAVE_LIBEDIT
    }
    #endif /* HAVE READLINE || HAVE_LIBEDIT */

    I think this is a good idea. Will do in a while.

    For reference:
    zgtmsg() is only called from
    src/exm/xpause.c
    src/exm/xnews.c
    src/cpf/cresp.c (Standard Error Recovery, not sure this is used)

    We don't need src/exm/xnews.c anymore, so I'm going to get rid of it
    -- one less thing to worry about. Is there anyway to get the dates of
    those sac2000 releases in file aux/news?

    There is a line in src/co/zgpmsg.c that reads:

    #if !defined(READLINE) && !defined(USE_X11_DOUBLE_BUFFER)

    I don't quite understand why USE_X11_DOUBLE_BUFFER has anything to do
    with line editing functionality. Could you please explain a bit?
    Thanks!


    Best regards,

    --
    Kuang He
    Department of Physics
    University of Connecticut
    Storrs, CT 06269-3046

    Tel: +1.860.486.4919
    Web: http://www.phys.uconn.edu/~he/

    • Brian Savage
      Re: readline/libedit patch
      2008-10-08 17:01:54
      Kuang He,

      The difference between zgpmsg() and zgtmsg() is zgpmsg() currently
      adds commands
      to the history while zgtmsg() does not. I imagine there is an easy
      way to handle this.

      xnews:
      I would suggest leaving the function in, but short-circuiting it:
      print a message and return.
      Look for the DEPRECATED function in src/dff/f2c.c, this may be
      useful. The information is stored
      in aux/news. We can probably change the filename to HISTORY.old and
      reference it in the HISTORY file
      and/or the README file.

      double_buffer:
      This was added to fix the window redrawing problem on some systems
      (Linux). It was added at the same time
      as the libedit/readline functionality which is why it they are stuck
      together. You are correct that the double_buffer
      has nothing do do with libedit functionality, but the select()
      function does. At the time of adding this function, it was either
      use two (2) cases or use (4) cases. I opted for two and lumped in
      READLINE with DOUBLE_BUFFER. I think we should
      be able to remove the DOUBLE_BUFFER option and force sac to always
      use the double buffer in future releases.

      Cheers
      Brian

      On Oct 7, 2008, at 9:23 PM, Kuang He wrote:

      On Tue, Oct 7, 2008 at 3:14 PM, Brian Savage <savage<at>uri.edu> wrote:
      I was looking at your readline patch to handle the real readline
      library
      when used with a script.
      http://www.iris.washington.edu/pipermail/sac-dev/2008-September/
      000109.html

      It looks like when the tty is not being used (use_tty()) you avoid
      the
      select_loop().
      This seems fine with me. We also have very similar code in the
      zgtmsg() and
      the code you added looks a lot like the original zgpmsg() code.

      Dear Brian,

      I've been meaning to ask you about the similar codes in zgtmsg.c and
      zgpmsg.c. How come we need two sets of those functions in the first
      place?

      Can we do some
      consolidation here, possibly just within the zgpmsg() to begin
      with. and
      include code
      when with HAVE_LIBEDIT or HAVE_READLINE ( currently just READLINE)
      is true.

      Ideally, we could totally get rid of the file zgtmsg.c, couldn't we?

      To make the the code easier to handle in the future I would prefer
      the
      libedit and
      readline behave the same way if a tty is, or is not, encountered.

      - Use the select_loop() while in a tty session.
      - Avoid the select_loop() while no tty is available
      - Avoid the select_loop() while no line handling is available

      How about something like:

      #ifdef HAVE_READLINE || HAVE_LIBEDIT
      if( use_tty() ) {
      select_loop()
      } else {
      #endif /* HAVE READLINE || HAVE_LIBEDIT */

      getfline()

      #ifdef HAVE_READLINE || HAVE_LIBEDIT
      }
      #endif /* HAVE READLINE || HAVE_LIBEDIT */

      I think this is a good idea. Will do in a while.

      For reference:
      zgtmsg() is only called from
      src/exm/xpause.c
      src/exm/xnews.c
      src/cpf/cresp.c (Standard Error Recovery, not sure this is used)

      We don't need src/exm/xnews.c anymore, so I'm going to get rid of it
      -- one less thing to worry about. Is there anyway to get the dates of
      those sac2000 releases in file aux/news?

      There is a line in src/co/zgpmsg.c that reads:

      #if !defined(READLINE) && !defined(USE_X11_DOUBLE_BUFFER)

      I don't quite understand why USE_X11_DOUBLE_BUFFER has anything to do
      with line editing functionality. Could you please explain a bit?
      Thanks!


      Best regards,

      --
      Kuang He
      Department of Physics
      University of Connecticut
      Storrs, CT 06269-3046

      Tel: +1.860.486.4919
      Web: http://www.phys.uconn.edu/~he/
      _______________________________________________
      sac-dev mailing list
      sac-dev<at>iris.washington.edu
      http://www.iris.washington.edu/mailman/listinfo/sac-dev


  • Kuang He
    Re: readline/libedit patch
    2008-10-10 02:36:08
    Dear Brian,

    Attached is a patch (on top of my original patches) that does the following:

    - define HAVE_LIBEDIT and HAVE_READLINE, and get rid of the original
    READLINE in configure.in
    - update corresponding source code due to the above change
    - Consolidate zgpmsg() implementations in src/co/zgpmsg.c
    - reverse part of a previous change in Makefile.in's that intends to
    get rid of duplicate -I../inc, which would also cause a compile
    failure however

    Best regards,

    --
    Kuang He
    Department of Physics
    University of Connecticut
    Storrs, CT 06269-3046

    Tel: +1.860.486.4919
    Web: http://www.phys.uconn.edu/~he/



    On Tue, Oct 7, 2008 at 3:14 PM, Brian Savage <savage<at>uri.edu> wrote:
    Kuang

    I was looking at your readline patch to handle the real readline library
    when used with a script.
    http://www.iris.washington.edu/pipermail/sac-dev/2008-September/000109.html

    It looks like when the tty is not being used (use_tty()) you avoid the
    select_loop().
    This seems fine with me. We also have very similar code in the zgtmsg() and
    the code you added looks a lot like the original zgpmsg() code. Can we do
    some
    consolidation here, possibly just within the zgpmsg() to begin with. and
    include code
    when with HAVE_LIBEDIT or HAVE_READLINE ( currently just READLINE) is true.
    To make the the code easier to handle in the future I would prefer the
    libedit and
    readline behave the same way if a tty is, or is not, encountered.

    - Use the select_loop() while in a tty session.
    - Avoid the select_loop() while no tty is available
    - Avoid the select_loop() while no line handling is available

    How about something like:

    #ifdef HAVE_READLINE || HAVE_LIBEDIT
    if( use_tty() ) {
    select_loop()
    } else {
    #endif /* HAVE READLINE || HAVE_LIBEDIT */

    getfline()

    #ifdef HAVE_READLINE || HAVE_LIBEDIT
    }
    #endif /* HAVE READLINE || HAVE_LIBEDIT */

    For reference:
    zgtmsg() is only called from
    src/exm/xpause.c
    src/exm/xnews.c
    src/cpf/cresp.c (Standard Error Recovery, not sure this is used)

    Attachments
SAGE Facility has been certified as a trustworthy data repository by the CoreTrustSeal Standards and Certification Board
10:24:26 v.22510d55