[m-rev.] for review: Allow --xml browse option from within declarative debugger

Julien Fischer juliensf at cs.mu.OZ.AU
Sat Feb 19 18:20:45 AEDT 2005


On Sat, 19 Feb 2005, Ian MacLarty wrote:

> For review by anyone.
>
> Estimated hours taken: 6
> Branches: main
>
> Allow an XML term browser to be called from the declarative debugger.
>
> browser/browse.m
> 	Add a predicate to save a term to an XML file and then launch an
> 	XML browser.
>
> 	Add a predicate that saves a term to an XML file and doesn't print
> 	any error messages, but just returns an io.res result and use this
> 	in save_term_to_file_xml and the predicate mentioned above.
>
Make the last bit a separate sentence.  e.g. Use this in  ...

> browser/browser_info.m
> 	Add two fields to the browser's persistent state - one to record the
> 	temporary filename to use when saving a term to an XML file and one
> 	to hold the command to launch the XML browser.  Previously these were
> 	stored in C global variables which were not accessable from the
s/accessable/accessible/

> 	declarative debugger.
>
> 	Export the browser_persistent_state type so the field access functions
> 	can be used from browse.m.
>
Is that necessary?  Why don't you just export the access functions for
the relevant fields?

> browser/declarative_user.m
> 	Allow the user to give an -x or --xml option to the browse command from
> 	within the declarative debugger.
>
> 	Reformat the user_command type and add a new functor: browse_xml_arg/1.
>
> 	Describe the -x or --xml browse option in the help message and reformat
> 	the trust command help message as it was looking a bit untidy.
>
> tests/debugger/browser_test.exp
> tests/debugger/browser_test.inp
> 	Test the --xml option from within the declarative debugger.
>
> trace/mercury_trace_browse.c
> trace/mercury_trace_browse.h
> trace/mercury_trace_internal.c
> 	Move MR_trace_save_and_invoke_xml_browser from mercury_trace_internal.c
> 	to mercury_trace_browse.c so it can call the new Mercury code in
> 	browser/browse.m.
>
> 	Handle the `set xml_browser_cmd' and `set xml_tmp_filename' commands by
> 	calling Mercury code to set the appropriate fields in the persistent
> 	browser state.
>
...

> @@ -145,11 +147,24 @@
>
>  %---------------------------------------------------------------------------%
>
> -	% An abstract data type that holds persistent browser settings.
> +	% An data type that holds persistent browser settings.
>  	% This state must be saved by the caller of the browse module
>  	% between calls.
>  	%

If only a few of these fields need to be exported, then exporting
the field acces functions may be a better choice than making the
type non-abstract.

> -:- type browser_persistent_state.
> +:- type browser_persistent_state
> +	--->	browser_persistent_state(
> +			print_params		:: caller_params,
> +			browse_params		:: caller_params,
> +			print_all_params	:: caller_params,
> +			num_printed_io_actions	:: int,
> +				% The command to lauch the user's
> +				% prefered XML browser.
> +			xml_browser_cmd		:: maybe(string),
> +				% The file to save XML to before
> +				% lauching the browser.
> +			xml_tmp_filename	:: maybe(string)
> +		).
> +
>

...

> @@ -427,6 +472,21 @@
>  	maybe_convert_dirs_to_path(MaybeDirs, MaybeMark),
>  	!:User = !.User ^ browser := Browser.
>
> +:- pred browse_xml_atom(trace_atom::in,	user_state::in, io::di, io::uo)
Why the odd spacing there?

> +	is cc_multi.
> +
> +browse_xml_atom(Atom, User, !IO) :-
> +	Atom = atom(ProcLayout, Args),
> +	ProcLabel = get_proc_label_from_layout(ProcLayout),
> +	get_user_arg_values(Args, ArgValues),
> +	get_pred_attributes(ProcLabel, Module, Name, _, PredOrFunc),
> +	Function = pred_to_bool(unify(PredOrFunc,function)),

I suggest calling that variable `IsFunction' rather than just `Function'.
Also put a space after the comma.

> +	sym_name_to_string(Module, ".", ModuleStr),
> +	BrowserTerm = synthetic_term_to_browser_term(ModuleStr ++ "." ++ Name,
> +		ArgValues, Function),
> +	save_and_browse_browser_term_xml(BrowserTerm, User ^ outstr,
> +		User ^ outstr, User ^ browser, !IO).
> +
>  :- func get_subterm_mode_from_atoms(trace_atom, trace_atom, list(dir))
>  	= browser_term_mode.

Looks good otherwise.

Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------



More information about the reviews mailing list