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

Ian MacLarty maclarty at cs.mu.OZ.AU
Sat Feb 19 20:56:10 AEDT 2005


On Sat, Feb 19, 2005 at 06:20:45PM +1100, Julien Fischer wrote:
> 
> 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  ...
> 

Righto.

> > 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/
> 

Fixed.

> > 	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?
> 

I didn't know that was possible.  How do I do that?

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

Is it just a case of putting:

:- func xml_browser_cmd(browser_persistent_state) = maybe(string).

in the interface?  What about the field update function?

> ...
> 
> > @@ -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?
> 

Strange - must be tabs instead of spaces.  It looks fine in my source
file.  I did a s/\t/ /g anyway.

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

Okay.

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

Cheers,

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