[m-dev.] for review: sorting variables better in the debugger

[Zoltan Somogyi]

> | (I can't easily test that myself).
> 
> Is there something I can do to make that easier in the future?

Provide a large set of automated test cases for Opium-M. If we had that,
I could do the testing myself, after installing Opium-M itself.

> | trace/mercury_trace_vars.[ch]:
> | 	A new module to manage the debugger's information about the variables
> | 	live at the current program point (which can be defined as the
> | 	combination of an specific event and an ancestor level).
> 
> s/an/a/

Done.

> | +extern	Word	mercury_data_private_builtin__type_ctor_info_type_info
       _1;
> | +extern	Word	mercury_data_private_builtin__type_ctor_info_type_ctor
       _info_1;
> | +extern	Word	mercury_data_private_builtin__type_ctor_info_typeclass
       _info_1;
> | +extern	Word	mercury_data_private_builtin__type_ctor_info_base_type
       class_info_1;
> 
> Fergus would say to keep lines < 80 columns ;)

Well, only one is actually over 79 columns, and in this case that is the
lesser of the short-term-available evils. The right fix is to globally change
the prefix from mercury_data_ to something shorter.

> | +static void
> | +MR_trace_browse_var(FILE *out, MR_Var_Details *var, MR_Browser browser)
> | +{
> | +	int	len;
> | +
> | +	if (out != NULL) {
> | +		/*
> | +		** The initial blanks are to visually separate
> | +		** the variable names from the prompt.
> | +		*/
> | +
> | +		fprintf(out, "%7s", "");
> | +		len = MR_trace_print_var_name(out, var);
> | +		while (len < MR_TRACE_PADDED_VAR_NAME_LENGTH) {
> | +			fputc(' ', out);
> | +			len++;
> | +		}
> 
> MR_trace_browse_var() is called from MR_trace_browse_one() which is called
> from MR_trace_browse_one_external(). The old MR_trace_browse_one_external()
> was only sending terms on the socket; I can't convince myself that a term
> will be sent
> here. Will it?

If the out arg is NULL, which it will be when it is called from the external
debugger, MR_trace_browse only invokes the supplied browser on the variable.
The supplied browser will be the same function your original code called,
MR_trace_browse_external; I did not modify that function at all.

> More generally, did you make sure all "the mostly common code" you factored
> out respect the fact that the external debugger process only reads terms on
> the socket?

I think so, but I could be wrong; that's what the review is for :-)

> enquery is not in my dictionnary. Maybe you mean enquiry?

Yes.

BTW, nor was Idem in mine.

> | +typedef struct {
> | +	MR_Var_Spec_Kind	MR_var_spec_kind;
> | +	int			MR_var_spec_number; /* valid if VAR_NUMBER */
> | +	const char		*MR_var_spec_name;  /* valid if VAR_NAME   */
> 
> s/VAR_NUMBER/MR_VAR_SPEC_NUMBER/
> s/VAR_NAME/MR_VAR_SPEC_NAME/

Actually, I just removed the VAR_ prefixes. Anything else was way messier,
for no real gain.

> | +extern	const char	*MR_trace_return_var_info(int n, const char **
       name_ptr,
> 
> This line exceeds 80 columns.

Actually, it doesn't. The extra "> " inserted by the mail system pushes the 
"extern" over the first tab stop.

> I am happy with that change as long as it doesn't break the external debugger
> (which i am not absolutely convinced though).
> 
> Maybe the simplest thing to do is to let you commit that change and then I
> will fix problems with the external debugger, if any exists.

I can do that, but I was asking you to apply the diff by hand in a spare
workspace and test whether it works for Opium-M before I commit it.

Zoltan.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------