Re: [PATCH] guile: Add 'history-push!' procedure

[Doug Evans <dje at google dot com>]

On Thu, Feb 20, 2014 at 2:29 PM, Ludovic CourtÃs <ludo@gnu.org> wrote:
> Eli Zaretskii <eliz@gnu.org> skribis:
>
>>> From: ludo@gnu.org (Ludovic CourtÃs)
>>> Cc: gdb-patches@sourceware.org,  xdje42@gmail.com
>>> Date: Wed, 19 Feb 2014 22:43:43 +0100
>>>
>>> >> +@deffn {Scheme Procedure} history-push! value
>>> >> +Push @var{value}, an instance of @code{<gdb:value>}, to @value{GDBN}'s
>>> >> +value history.
>>> >
>>> > I think we should explain what "push" means in this context.  It is
>>> > not self-evident.
>>>
>>> Right.  How about s/push/append/?  (In the procedure name as well.)
>>
>> Fine with me.
>
> Done.
>
>>> >>                 Return its index in the history, or @code{#f} if it is
>>> >> +not saved.
>>> >
>>> > Why would it not be saved?
>>>
>>> Good question.  The doc above ârecord_latest_valueâ mentions that it can
>>> return -1, but I see no path leading to that.  Can I assume that -1 is
>>> never returned?
>>
>> I don't know, but if that is your reading, I see no reason not to.
>
> OK, I changed it to assume it always returns a positive value.
>
>>> How about simply this:
>>>
>>>   Using this function is useful to provide convenient access to values
>>>   manipulated by a Guile extension of GDB.
>>
>> I suggest
>>
>>   Putting into history values returned by Guile extensions will allow
>>   the user convenient access to those values via CLI history
>>   facilities.
>
> Perfect.
>
> The version below incorporates those changes.
>
> Thanks,
> Ludoâ.
>
> gdb/
> 2014-02-21  Ludovic CourtÃs  <ludo@gnu.org>
>
>         * guile/scm-value.c (gdbscm_history_append_x): New function.
>         (value_functions): Add it.
>
> gdb/testsuite/
> 2014-02-21  Ludovic CourtÃs  <ludo@gnu.org>
>
>         * gdb.guile/scm-value.exp (test_value_in_inferior): Add
>         test for 'history-append!'.
>
> gdb/doc/
> 2014-02-21  Ludovic CourtÃs  <ludo@gnu.org>
>
>         * gdb/doc/guile.texi (Basic Guile): Document 'history-append!'.

LGTM.

Thanks!