Add some other functions to the readline module.
See discussion on clisp-users list
Logged In: YES
1. please do not omit (:return-type nil)
2. please do not omit (:arguments)
3. please include the :alloc fixes for add-defun and
4. please include a ChangeLog entry
also, please make sure that all functions you add to
readline.lisp are checked in readline/configure.in
Logged In: YES
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
5. I guess tests are welcome
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
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
1. changes should include date, your name, and your e-mail.
2. please make sure that your patch is against a recent CVS
configure.in patch will _not_ apply cleanly!
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.
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
when Readline is waiting for terminal input. By
will be called at most ten times a second if there is
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
4. OK. Changed vim settings for lisp files, so this should not
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)
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?
2. anoncvs is updated with a certain delay wrt devel cvs
5. doc strings: 80 columns if possible.
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.
5. I will try for functions/variables, but I believe longer
package documentation could be acceptable. I think this
requirement should be added to the CodingStyle. Also, I
guess you mean 78, or possibly 77 chars, to fit the row even
6. Thanks, that is exactly what I looked for. I saw your
discussion on c.l.l. - do you want to have clear-input
modified to also clear the readline buffer? If so, I miss a hook
no requirement is absolute.
I prefer that lines do not wrap, when possible.
don't worry about clear-input issues - at least for now.
looks good except for tiny nits that I can fixed as I apply
(e.g., I would suggest naming the lambda and using
are you done?
I think I am done for the moment. Let me reiterate that signal
handling and completion functions are still missing, but I have
no idea how would I use/test them - it appears that Yaroslav
is trying to make the signal handling part work. I would prefer
to have this included now.
Thanks for pushing in right directions.
committed -- thanks!