Re: PATCH: operate-and-get-next

Elena Zannoni
2001-10-10T17:53:00Z

Tom Tromey writes:
> Bash has a readline command called `operate-and-get-next' which I use
> very frequently (many times per day). It works like this: go up the
> history list using C-p. Then type C-o (the operate-and-get-next key).
> This acts like Enter, but when the prompt returns the next command in
> history is already available for editing. This key binding makes it
> very convenient to replay a previously-entered sequence of commands.
> I've long missed it in gdb; for some reason it is a bash-specific key
> binding and is not available in readline itself.
>

Yes, I like this feature.

> The appended patch implements this new readline command and gives it
> the same binding in gdb that it has in bash.
>
> The implementation is a little ugly (it adds a couple new global
> variables). It is the fourth or fifth try. Anything more obvious (to
> me) didn't work out. You can see some of the reasoning in the
> comments in rl_callback_read_char_wrapper.
>
> Built and tested on x86 Red Hat Linux 6.2. I tested it interactively,
> with both --async and --noasync. I also ran the test suite with no
> regressions.
>
> Is this ok to commit?
>

It looks to me that prompt_just_set is always 1, after it is set the
very first time. So it doesn't really help. If I get this right, your
problem is that readline needs to somehow retain the memory of the
fact that it was executing a cntrl-o before returning control to gdb,
and that it needs to complete its job, by displaying the next command
in the history. (This is very similar to the same problem the gdb is
facing of remembering what command it was executing and for which I
had to introduce the 'continuation' mechanism.)
Instead of having this global variable, could it be possible to play with
installing and un-installing the hook itself?
And then have rl_callback_read_char_wrapper do a check

if (blah_hook)
blah_hook();

Actually I would like to make bla_hook be a generic function pointer,
and assign to this a function with a more specific name like
'operate_and_get_next_completion' or something of the sort.

Also, by setting the variable (or the hook) in gdb_display_prompt, we
do this call every time we process one single char, not only after a
cntrl-o was issued. It is the body of the hook function that knows
whether or not a cntrl-o was done because of the operate_saved_history
value not being -1.

Could we instead set this hook in the operate_and_get_next function
itself? And call it at the same spot you call it now.

In the same way the rl_pre_input_hook could also be set in the same
operate-and-get-next function. You can even use event_loop_p in there
to determine what hook to set.

We must also find a way to deinstall the hook after the whole cntrl-o
has completed.

thanks
Elena

> Tom
>
> Index: ChangeLog
> from Tom Tromey <tromey@redhat.com>
>
> * command.h (gdb_rl_pre_input_hook): Declare.
> * event-top.c (display_gdb_prompt): Set prompt_just_set.
> (prompt_just_set): New global.
> (rl_callback_read_char_wrapper): Call gdb_rl_pre_input_hook if
> required.
> * top.c (operate_saved_history): New global.
> (gdb_rl_operate_and_get_next): New function.
> (gdb_rl_pre_input_hook): New function.
> (init_main): Set rl_pre_input_hook and add the
> operate-and-get-next defun.
>
> Index: command.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/command.h,v
> retrieving revision 1.18
> diff -u -r1.18 command.h
> --- command.h 2001/07/16 14:46:34 1.18
> +++ command.h 2001/10/03 22:41:30
> @@ -350,6 +350,8 @@
>
> extern NORETURN void error_no_arg (char *) ATTR_NORETURN;
>
> +extern void gdb_rl_pre_input_hook ();
> +
> extern void dont_repeat (void);
>
> /* Used to mark commands that don't do anything. If we just leave the
> Index: event-top.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/event-top.c,v
> retrieving revision 1.16
> diff -u -r1.16 event-top.c
> --- event-top.c 2001/08/27 22:39:55 1.16
> +++ event-top.c 2001/10/03 22:41:31
> @@ -153,6 +153,11 @@
> char *linebuffer_ptr;
> }
> readline_input_state;
> +
> +/* This is used to communicate from the prompt machinery back to the
> + character handler. See rl_callback_read_char_wrapper for more
> + information. */
> +static int prompt_just_set = 0;
> \f
>
> /* Wrapper function for calling into the readline library. The event
> @@ -162,6 +167,19 @@
> rl_callback_read_char_wrapper (gdb_client_data client_data)
> {
> rl_callback_read_char ();
> +
> + /* If the prompt was just set, see if we need to do some work for
> + the operate-and-get-next key binding.
> + We can't use rl_pre_input_hook for this, because the callback-driven
> + readline clears the current command line after this hook is
> + called. We can't put this code directly in display_gdb_prompt,
> + because that is called from a callback before the current command
> + line is cleared in readline. So instead we arrange for a flag to
> + be set at the right moment; if this flag is set after we process
> + a character then we know we've just printed a new prompt and we
> + are now ready to do any pre-input processing. */
> + if (prompt_just_set)
> + gdb_rl_pre_input_hook ();
> }
>
> /* Initialize all the necessary variables, start the event loop,
> @@ -293,6 +311,9 @@
> {
> rl_callback_handler_remove ();
> rl_callback_handler_install (new_prompt, input_handler);
> +
> + /* See rl_callback_read_char_wrapper to understand this. */
> + prompt_just_set = 1;
> }
> /* new_prompt at this point can be the top of the stack or the one passed in */
> else if (new_prompt)
> Index: top.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/top.c,v
> retrieving revision 1.45
> diff -u -r1.45 top.c
> --- top.c 2001/09/07 21:33:08 1.45
> +++ top.c 2001/10/03 22:41:33
> @@ -1032,6 +1032,41 @@
> #endif
> }
> \f
> +/* The current saved history number from operate-and-get-next.
> + This is -1 if not valid. */
> +static int operate_saved_history = -1;
> +
> +/* This is a gdb-local readline command handler. It accepts the
> + current command line (like RET does) and, if this command was taken
> + from the history, arranges for the next command in the history to
> + appear on the command line when the prompt returns.
> + We ignore the arguments. */
> +static int
> +gdb_rl_operate_and_get_next (int count, int key)
> +{
> + /* Add 1 because we eventually want the next line. */
> + operate_saved_history = where_history () + 1;
> + return rl_newline (1, key);
> +}
> +
> +/* This function is put on the `rl_pre_input_hook' and helps
> + operate-and-get-next do its work. */
> +void
> +gdb_rl_pre_input_hook ()
> +{
> + if (operate_saved_history != -1)
> + {
> + int delta = where_history () - operate_saved_history;
> + /* The `key' argument to rl_get_previous_history is ignored. */
> + rl_get_previous_history (delta, 0);
> + operate_saved_history = -1;
> +
> + /* readline doesn't automatically update the display for us. */
> + rl_redisplay ();
> + }
> +}
> +
> +\f
> /* Read one line from the command input stream `instream'
> into the local static buffer `linebuffer' (whose current length
> is `linelength').
> @@ -1847,6 +1882,10 @@
> if (!event_loop_p)
> {
> gdb_prompt_string = savestring (DEFAULT_PROMPT, strlen (DEFAULT_PROMPT));
> +
> + /* This hook only works correctly when we are using the
> + synchronous readline. */
> + rl_pre_input_hook = (Function *) gdb_rl_pre_input_hook;
> }
> else
> {
> @@ -1881,6 +1920,10 @@
> get_gdb_completer_word_break_characters ();
> rl_completer_quote_characters = get_gdb_completer_quote_characters ();
> rl_readline_name = "gdb";
> +
> + /* The name for this defun comes from Bash, where it originated.
> + 15 is Control-o, the same binding this function has in Bash. */
> + rl_add_defun ("operate-and-get-next", gdb_rl_operate_and_get_next, 15);
>
> /* The set prompt command is different depending whether or not the
> async version is run. NOTE: this difference is going to