octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Updated patch for multiple users of readline event hook


From: John Swensen
Subject: Re: Updated patch for multiple users of readline event hook
Date: Tue, 18 Sep 2007 08:19:37 -0400
User-agent: Thunderbird 2.0.0.6 (Macintosh/20070728)

John W. Eaton wrote:
On 17-Sep-2007, John Swensen wrote:

| I had originally submitted a patch that left these files as C files. | However, in that version I implemented the callback list as a simple C | array with a 256 element array of function pointers. Shai Ayal was the | only one who gave feedback on the original patch and suggested I use | std::vector/C++, rather than a fixed-size array. | | Either one works for me. Which would you prefer?

I think the correct place for this change is in
liboctave/cmd-edit.cc.  The logic for having a list of event hook
functions should be independent of the actual class that implements
command editing (gnu_readline, default_command_editor, ...).

Also, your patch introduces some pthread things and includes a file
"pthread.h" which I don't think you supplied.  It seems to me that
adding that is a separate issue from adding a list of hook functions,
so please submit a patch that simply makes it possible for the

  command_editor::set_event_hook (event_hook_fcn f);
  command_editor::restore_event_hook (void);

functions to (though I think that these should perhaps be renamed to
add_event_hook and remove_event_hook), and that only makes that
change.  Perhaps the startup hook function could be handled in a
simmilar way for consistency.

Thanks,

jwe

OK...after looking at my implementation and how you suggest, yours is much better. I have started working on it, but I do have one more question though. I still need to create a "default" handler for both the startup hook and the event hook that then calls each of the registered functions. Would you prefer this to be in the cmd-edit.cc file also? It won't be a member of the class, since you can't set the readline function pointer to a class function (not even a static one).

John Swensen


reply via email to

[Prev in Thread] Current Thread [Next in Thread]