[m-dev.] For review: Incorporate the term brower stuff in the external debugger. [part 2/2]

[Fergus Henderson]

On 20-May-1999, Erwan Jahier <Erwan.Jahier at irisa.fr> wrote:
> Index: browser/debugger_interface.m
...
> +:- pred get_variable_name(debugger_request, string).
> +:- mode get_variable_name(in, out) is det.
> +
> +:- pragma export(get_variable_name(in, out), "ML_DI_get_variable_name").
> +	% Predicate that allows to retrieve the name of the variable to browse
> +	% from a `browse(var_name)' request in mercury_trace_external.c.

That should be "% Predicate that allows mercury_trace_external.c to
retrieve ... from a `browse(var_name)' request."

Also it would be better to replace "Predicate that allows ..." with
"This predicate allows ...".

> +++ mercury_layout_util.c	1999/05/20 12:03:25
> +/*
> +** Find and validate the number of a variable given by a variable
> +** specification in the given layout. If successful, return the
> +** number of the variable in *which_var_ptr, and a NULL string;

I suggest s/return/store/ and s/a NULL/return a NULL/

Likewise for the same comment in mercury_layout_util.h.

> +++ mercury_trace_browse.c	1999/05/20 12:03:27
> @@ -63,6 +63,28 @@
>  				(Word *) MR_trace_browser_state_type);
>  }
>  
> +	
> +/*
> +** MR_trace_browse_external() is the same as MR_trace_browse() except it 
> +** uses debugger_socket_in and debugger_socket_out instead of mdb_in and 
> +** mdb_out.
> +*/

This comment is a bit misleading, I think.  If that were they only 
difference, then the two functions should be combined into one.
But there is another difference, and that is that MR_trace_browse_external()
does its I/O in program-readable terms, whereas MR_trace_browse() does it
in human-readable strings.

> --- mercury_trace_external.c	1999/04/30 06:21:47	1.16
> +++ mercury_trace_external.c	1999/05/20 12:03:32
> @@ -390,6 +400,7 @@
>  	Word		*saved_regs = event_info->MR_saved_regs;
>  	Integer		modules_list_length;
>  	Word		modules_list;
> +	int		ancestor_level;
>  
>  /* 
>  ** MR_mmc_options contains the options to pass to mmc when compiling queries.
> @@ -398,6 +409,9 @@
>  	static String	MR_mmc_options;
>  	MR_TRACE_CALL_MERCURY(ML_DI_init_mercury_string(&MR_mmc_options));
>  
> +/* by default, print variables from the current call */
> +	ancestor_level = 0;

The comment should be indented the same amount as the code that it
refers to.

> +/*
> +** It is the same function as MR_trace_browse_one() defined in 
> +** mercury_trace_internal.c except it sends/receives messages from/to 
> +** the socket instead of sending it to mdb_in/mdb_out.
> +*/
> +static void
> +MR_trace_browse_one_external(const MR_Stack_Layout_Label *top_layout,

s/It is the same function as/This function does the same thing as/
s at sending it to at sending them from/to@

Also the comment here is misleading. If that were they only 
difference, then the two functions should be combined into one.
But there is another difference, and that is that This one
does its I/O in program-readable terms, whereas MR_trace_browse_one() does it
in human-readable strings.

> +/*
> +** It is the same function as MR_trace_browse_var() defined in 
> +** mercury_trace_internal.c except it always calls the term browser and it calls 
> +** MR_trace_browse_external() instead of MR_trace_browse().
> +*/
> +
> +static void
> +MR_trace_browse_var_external(const char *name, const MR_Stack_Layout_Vars *vars,

s/It is the same function as/This function does the same thing as/

Please keep lines < 80 columns.

--------------------

Apart from the issues mentioned above, your change looks good.
I would like to see a relative diff when you have addressed those
issues.  Thanks.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------