Re: [patch] Add visible flag to breakpoints.

[Eli Zaretskii <eliz at gnu dot org>]

> From: Phil Muldoon <pmuldoon@redhat.com>
> Cc: Pedro Alves <pedro@codesourcery.com>, gdb-patches@sourceware.org,        dan@codesourcery.com
> Date: Fri, 22 Oct 2010 22:05:30 +0100
> 
> I decided to add internal watchpoint support.  I've also added the other
> changes requested.
> 
> OK?

Thanks.

> -/* Set a breakpoint.  This function is shared between CLI and MI
> +/* Set a breakpoint. This function is shared between CLI and MI

Why did you need to remove one of the two spaces between these
sentences?

>     functions for setting a breakpoint.  This function has two major
>     modes of operations, selected by the PARSE_CONDITION_AND_THREAD
>     parameter.  If non-zero, the function will parse arg, extracting
>     breakpoint location, address and thread. Otherwise, ARG is just the
>     location of breakpoint, with condition and thread specified by the
> -   COND_STRING and THREAD parameters.  Returns true if any breakpoint
> -   was created; false otherwise.  */
> -
> +   COND_STRING and THREAD parameters.  If INTERNAL is non-zero, the
> +   breakpoint number will be allocated from the internal breakpoint
> +   count.   Returns true if any breakpoint was created; false
            ^^^
And here you have 3 instead of 2.

> +@code{maint info breakpoints} command).  The optional @var{wp_class}
>  argument defines the class of watchpoint to create, if @var{type} is
>  defined as @code{BP_WATCHPOINT}.

"if @var{type} is @code{BP_WATCHPOINT}" is simpler and more clear, IMO.

> +@defivar Breakpoint visible
> +This attribute holds the breakpoint's visibility flag---the identifier used to
> +determine whether the breakpoint is visible to the user when set, or
> +when the @samp{info breakpoints} command is run.

There's no need to go through identifiers.  How about this rewording?

  This attribute tells whether the breakpoint is visible to the user
  when set, or when the @samp{info breakpoints} command is run.

Thanks.