Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#17 More functions in readline module

closed-accepted
Sam Steingold
None
5
2005-10-17
2005-10-03
Tomas Zellerin
No

Add some other functions to the readline module.

See discussion on clisp-users list
http://article.gmane.org/gmane.lisp.clisp.general/10192

Discussion

1 2 > >> (Page 1 of 2)
  • Sam Steingold
    Sam Steingold
    2005-10-03

    Logged In: YES
    user_id=5735

    1. please do not omit (:return-type nil)
    2. please do not omit (:arguments)
    3. please include the :alloc fixes for add-defun and
    readline-name
    4. please include a ChangeLog entry
    thanks!

     
  • Sam Steingold
    Sam Steingold
    2005-10-03

    Logged In: YES
    user_id=5735

    also, please make sure that all functions you add to
    readline.lisp are checked in readline/configure.in

     
  • Tomas Zellerin
    Tomas Zellerin
    2005-10-04

    Logged In: YES
    user_id=120720

    1. Style questions (return-type and arguments): OK
    2. Changelog entry: text in comment here is enough? I dont
    feel like hunting last version of ChangeLog to diff against.
    3. configure.in: really all functions? Where? In
    readline/configure.in directly (I prefer), add to CL_READLINE,
    create separate macro?
    4. Any FAQ that would answer questions like these? (I saw
    CodingStyle)
    5. I guess tests are welcome

     
  • Sam Steingold
    Sam Steingold
    2005-10-06

    Logged In: YES
    user_id=5735

    2. changelog entries are never submitted as a diff - always
    as a complete stand-alone entry

    3. see modules/readline/configure.in

    4. codingstyle is the official place. what's missing?

    5. yes, please see modules/readline/test.tst

    thanks!

     
  • Tomas Zellerin
    Tomas Zellerin
    2005-10-07

    Logged In: YES
    user_id=120720

    What about this?

    Changes are on top of the patch file.
    I still have some open problems (search for !!!).
    I do not exactly know where test infrastructure should be
    placed.

     
  • Sam Steingold
    Sam Steingold
    2005-10-07

    Logged In: YES
    user_id=5735

    1. changes should include date, your name, and your e-mail.

    2. please make sure that your patch is against a recent CVS
    head.
    configure.in patch will _not_ apply cleanly!

    3. !!!:
    a.rl_ funmap_names: I think you need (c-array-max :malloc-free).
    if it does not work, please ask clisp-list (CC joerg)
    b. rl_message: one can always do formatting in lisp, so your
    definition is just fine - just note in the doc string that
    the text may
    not have % in it.
    c. rl_reset_terminal: why does it leak? I don't think your
    need :malloc-free there, the default (:alloca) should work.

    4. please avoid #\Tab characters

    5. please try to keep your lines to 80 characters wide.

    6. full-line, position, line-length should be slots in the
    readline-stream class - BTW, great idea!!

    7. test-help should be in test.tst, see, e.g., rawsock/test.tst
    note that there is already test.tst in the CVS.

    thanks!

     
  • Sam Steingold
    Sam Steingold
    2005-10-09

    Logged In: YES
    user_id=5735

    please don't forget this one - it appears very useful!

    - Variable: rl_hook_func_t * rl_event_hook
    If non-zero, this is the address of a function to call
    periodically
    when Readline is waiting for terminal input. By
    default, this
    will be called at most ten times a second if there is
    no keyboard
    input.

     
  • Tomas Zellerin
    Tomas Zellerin
    2005-10-11

    Logged In: YES
    user_id=120720

    0. rl-event-hook is in all versions of patch and is even
    mentioned in packages documentation
    1. OK. I hope the date wont be awfully wrong.
    2. Is it possible that CVS browsing is sometimes out of
    sync? I am pretty sure that when I checked it in Thursday or
    Friday, changes were not propagated there. This patch is
    against Mondey morning CEST
    3. Unless it is recent change, I do not see how use c-array-
    max :malloc-free (and I do not need max, it is unknown size
    NULL terminated).
    4. OK. Changed vim settings for lisp files, so this should not
    happen again.
    5. In general, I do. However,
    - I guess that you want to have on one line function
    name, :name and untested string. This in few cases goes
    over 80 chars.
    - What is policy for long :documentation string splitting?
    6. I admittedly do not like singleton classes. IMHO the right
    way is to define a class that would implement buffered input
    to character input generally, but I do not consider readline
    right place for it. Also, it must have been solved somewhere
    in the core clisp package, too, and possibly could be
    merged. I would prefer to let it be at the moment and later
    cleanup when one knows what is the right way.
    7. I did not realize in rawsock how to define a function that
    can be used in several tests. The auxiliary function is
    streamlined and moved to readline.lisp for the time being.
    Also, looking at your tests, in my implementation are foreign
    variables not bound - is it a recent change? (I should possibly
    check out all the clisp, not just readline)

     
  • Tomas Zellerin
    Tomas Zellerin
    2005-10-11

    Logged In: YES
    user_id=120720

    2. Just tried, now the patch applies well against cvs
    8. only curious - when you set rl_readline_name in stream.d,
    why not use strdup? Not portable enough?

     
  • Sam Steingold
    Sam Steingold
    2005-10-11

    Logged In: YES
    user_id=5735

    2. anoncvs is updated with a certain delay wrt devel cvs

    5. doc strings: 80 columns if possible.

    6. http://clisp.cons.org/impnotes/stream-dict.html#stream-buffer

    7. modules/rawsock/test.tst has some defuns

    8. i have never heard of strdup before. portability does
    matter: it is never used in CLISP, so using it here would
    add a risk.

     
1 2 > >> (Page 1 of 2)