[m-dev.] for review: new browser parameters

[Mark Anthony BROWN]

Zoltan Somogyi writes:
> On 19-Oct-2000, Mark Anthony BROWN <dougl at cs.mu.OZ.AU> wrote:
> > browser/browser_info.m:
> > 	New module which defines the key browser data structures.  The
> > 	`browser_state' previously used by browse.m has been split into
> > 	two parts: the transient state (browser_info) and the persistent
> > 	state (browser_state).  The persistent state is created when the
> > 	browser is first called, and is saved between calls to the browser,
> > 	whereas a new transient state is created each time the browser is
> > 	called, and lasts for the duration of the call.
> 
> The name browser_state suggests something transient to me. We probably
> shouldn't use browser_params, since the values stored in it can be overridden.
> How about browser_preferences?

Hmm, it might be used to hold more than preferences in future.  For example,
it could store subterms selected by the user, for use in later interactive
queries.

How about browser_persistent_state?

> 
> > @@ -353,13 +307,13 @@
> >  "\tquit           -- quit browser\n",
> >  "SICStus Prolog style commands are:\n",
> >  "\tp              -- print\n",
> > -"\t< [n]          -- set depth (default is ", DefaultStr, ")\n",
> > +"\t< [n]          -- set depth\n",
> 
> What happens now if you don't supply a number?

It uses the old system default, because I forgot to update that part.
I've fixed it by making the argument non-optional -- at this stage it
doesn't make sense to use a system default because there are several of
them and no way to choose which one to apply.

> 
> > +	% The maximum estimated size for which we use `io__write'.
> > +:- pred max_print_size(int::out) is det.
> > +max_print_size(60).
> 
> Any reason for not using a function?

That code was moved from elsewhere.

> 
> > +	% Estimate the total term size, in characters.
> > +	% We count the number of characters in the functor,
> > +	% plus two characters for each argument: "(" and ")"
> > +	% for the first, and ", " for each of the rest,
> > +	% plus the sizes of the arguments themselves.
> > +	% This is only approximate since it doesn't take into
> > +	% account all the special cases such as operators.
> > +:- pred term_size(univ::in, int::out) is det.
> > +term_size(Univ, TotalSize) :-
> > +	deconstruct(Univ, Functor, Arity, Args),
> > +	string__length(Functor, FunctorSize),
> > +	list__map(term_size, Args, ArgSizes),
> > +	AddSizes = (pred(X::in, Y::in, Z::out) is det :- Z = X + Y),
> > +	list__foldl(AddSizes, ArgSizes, Arity * 2, TotalArgsSize),
> > +	TotalSize = TotalArgsSize + FunctorSize.
> 
> Is this still used anywhere? Please make sure it isn't, since it can take
> a looonnnggg time on large terms.
> 

It's not used, and I have removed it.

> > +The browser also maintains separate configuration parameters
> > +for the three different output formats.
> > +The options @samp{-f} or @samp{--flat}, @samp{-p} or @samp{--pretty},
> > +and @samp{-v} or @samp{--verbose}
> > +select which formats will be affected by the change.
> > +If none of these options is given,
> > +the default is to affect all formats.
> 
> You should mention that the one exception from this is if you set
> the format itself.

Ok.

> 
> > +static	bool		MR_trace_is_portray_format(const char *str,
> > +				MR_Browse_Format *format);
> 
> This indentation looks incorrect.
> 

It looks ok in the actual file (I just checked).  The other places you
mentioned are ok, too.

> > +	else if (streq(param, "format") &&
> > +			MR_trace_is_portray_format(value, &new_format))
> > +	{
> > +		MR_TRACE_CALL_MERCURY(
> > +			ML_BROWSE_set_param_format(print, browse, print_all,
> > +				new_format, MR_trace_browser_state,
> > +				&MR_trace_browser_state);
> > +		);
> 
> I would put this code either first or last in the chain of if-then-elses,
> since it is somewhat different from the others.

Good idea.

> 
> > browser_info__get_format(Info, Caller, MaybeFormat, Format) :-
> > 	(
> > 		MaybeFormat = yes(Format)
> > 	;
> > 		MaybeFormat = no,
> > 		Override = Info ^ format,
> > 		(
> > 			Override = yes(Format)
> > 		;
> > 			Override = no,
> > 			get_caller_params(Info ^ state, Caller, Params),
> > 			Format = Params ^ default_format
> > 		)
> > 	).
> 
> The variable Override is misnamed, since it doesn't override the parameter
> supplied by the user for this command.

I've renamed it MdbFormatOption.

> 
> > 	% Initialise the persistent settings with default values.  The
> > 	% rationale for the default values is:
> > 	% 	Depth and Size:
> > 	%		For non-interactive display, these are 3 and 10 resp.,
> > 	%		so that terms will generally fit on one line.  For
> > 	%		interactive browsing these values are increased.
> > 	%
> > 	%	Width:
> > 	%		Defaults to 80 characters in any situation.
> > 	%
> > 	%	Lines:
> > 	%		If one term is printed then it is limited to 25 lines.
> > 	%		If there can be more than one term (i.e., with
> > 	%		`print *') then a much lower limit is imposed.  For
> > 	%		verbose format, there is not much point setting this to
> > 	%		less than about 5 since otherwise very little of the
> > 	%		term will be shown.
> 
> I think the default depth and size limits for printing one variable should be
> significantly greater than 3 and 10; they should probably be the same as the
> corresponding defaults for browsing.
> 

Those values are the ones we currently use, and I'm not so sure about
changing them.  Anyone else care to comment?

Here is a relative diff of the above changes, plus the other things you
mentioned.  I'll commit this when we think of a good name to replace
`browser_state'.  The system defaults are easy to change -- that can
be done after some feedback from other users.

Cheers,
Mark.

--- log.old	Fri Oct 27 14:52:27 2000
+++ log	Fri Oct 27 15:00:33 2000
@@ -61,6 +61,11 @@
 	Move the definitions of `dir', `setting', and `portray_format'
 	to the interface of the new module.
 
+	The argument to the `<' command, which sets the depth limit, is
+	no longer optional.  The default used to be to use the system
+	default, but now there are multiple system defaults and no way to
+	select which one.
+
 browser/*.m:
 tests/debugger/browse_pretty.inp:
 tests/debugger/browse_pretty.exp:
--- browser/browse.m.old	Fri Oct 27 01:51:46 2000
+++ browser/browse.m	Fri Oct 27 14:33:50 2000
@@ -307,8 +307,8 @@
 "\tquit           -- quit browser\n",
 "SICStus Prolog style commands are:\n",
 "\tp              -- print\n",
-"\t< [n]          -- set depth\n",
-"\t^ [path]       -- cd [path]\n",
+"\t< n            -- set depth\n",
+"\t^ [path]       -- cd [path] (default is root)\n",
 "\t?              -- help\n",
 "\th              -- help\n",
 "\n",
@@ -400,22 +400,6 @@
 	% The maximum estimated size for which we use `io__write'.
 :- pred max_print_size(int::out) is det.
 max_print_size(60).
-
-	% Estimate the total term size, in characters.
-	% We count the number of characters in the functor,
-	% plus two characters for each argument: "(" and ")"
-	% for the first, and ", " for each of the rest,
-	% plus the sizes of the arguments themselves.
-	% This is only approximate since it doesn't take into
-	% account all the special cases such as operators.
-:- pred term_size(univ::in, int::out) is det.
-term_size(Univ, TotalSize) :-
-	deconstruct(Univ, Functor, Arity, Args),
-	string__length(Functor, FunctorSize),
-	list__map(term_size, Args, ArgSizes),
-	AddSizes = (pred(X::in, Y::in, Z::out) is det :- Z = X + Y),
-	list__foldl(AddSizes, ArgSizes, Arity * 2, TotalArgsSize),
-	TotalSize = TotalArgsSize + FunctorSize.
 
 term_size_left_from_max(Univ, MaxSize, RemainingSize) :-
 	( MaxSize < 0 ->
--- browser/browser_info.m.old	Fri Oct 27 03:01:29 2000
+++ browser/browser_info.m	Fri Oct 27 04:02:01 2000
@@ -21,14 +21,16 @@
 	%
 :- type browser_info
 	--->	browser_info(
-			term	:: univ,		% Term to browse.
-			dirs	:: list(dir),		% Root rel `present
-							% working directory'.
+			term	:: univ,	% Term to browse.
+			dirs	:: list(dir),	% The list of directories to
+						% take, starting from the root,
+						% to reach the current subterm.
 			format	:: maybe(portray_format),
-							% Format specified as
-							% an option to the mdb
-							% command.
-			state	:: browser_state	% Persistent settings.
+						% Format specified as
+						% an option to the mdb
+						% command.
+			state	:: browser_state
+						% Persistent settings.
 		).
 
 :- type dir
@@ -68,8 +70,7 @@
 	;	size(int)
 	;	format(portray_format)
 	;	width(int)
-	;	lines(int)
-	.
+	;	lines(int).
 
 	% Initialise a new browser_info.  The optional portray_format
 	% overrides the default format.
@@ -108,9 +109,9 @@
 	% indicate the presence of the `set' options -P, -B, -A, -f, -p,
 	% and -v, in that order.
 	%
-:- pred browser_info__set_param(bool, bool, bool, bool, bool, bool, setting,
-		browser_state, browser_state).
-:- mode browser_info__set_param(in, in, in, in, in, in, in, in, out) is det.
+:- pred browser_info__set_param(bool::in, bool::in, bool::in, bool::in,
+		bool::in, bool::in, setting::in, browser_state::in,
+		browser_state::out) is det.
 
 %---------------------------------------------------------------------------%
 
@@ -124,45 +125,44 @@
 	% call browser_info__set_param from C code.
 	%
 
-:- pred set_param_depth(bool, bool, bool, bool, bool, bool, int,
-		browser_state, browser_state).
-:- mode set_param_depth(in, in, in, in, in, in, in, in, out) is det.
+:- pred set_param_depth(bool::in, bool::in, bool::in, bool::in, bool::in,
+		bool::in, int::in, browser_state::in, browser_state::out)
+		is det.
 :- pragma export(set_param_depth(in, in, in, in, in, in, in, in, out),
 		"ML_BROWSE_set_param_depth").
 
 set_param_depth(P, B, A, F, Pr, V, Depth) -->
 	browser_info__set_param(P, B, A, F, Pr, V, depth(Depth)).
 
-:- pred set_param_size(bool, bool, bool, bool, bool, bool, int,
-		browser_state, browser_state).
-:- mode set_param_size(in, in, in, in, in, in, in, in, out) is det.
+:- pred set_param_size(bool::in, bool::in, bool::in, bool::in, bool::in,
+		bool::in, int::in, browser_state::in, browser_state::out)
+		is det.
 :- pragma export(set_param_size(in, in, in, in, in, in, in, in, out),
 		"ML_BROWSE_set_param_size").
 
 set_param_size(P, B, A, F, Pr, V, Size) -->
 	browser_info__set_param(P, B, A, F, Pr, V, size(Size)).
 
-:- pred set_param_width(bool, bool, bool, bool, bool, bool, int,
-		browser_state, browser_state).
-:- mode set_param_width(in, in, in, in, in, in, in, in, out) is det.
+:- pred set_param_width(bool::in, bool::in, bool::in, bool::in, bool::in,
+		bool::in, int::in, browser_state::in, browser_state::out)
+		is det.
 :- pragma export(set_param_width(in, in, in, in, in, in, in, in, out),
 		"ML_BROWSE_set_param_width").
 
 set_param_width(P, B, A, F, Pr, V, Width) -->
 	browser_info__set_param(P, B, A, F, Pr, V, width(Width)).
 
-:- pred set_param_lines(bool, bool, bool, bool, bool, bool, int,
-		browser_state, browser_state).
-:- mode set_param_lines(in, in, in, in, in, in, in, in, out) is det.
+:- pred set_param_lines(bool::in, bool::in, bool::in, bool::in, bool::in,
+		bool::in, int::in, browser_state::in, browser_state::out)
+		is det.
 :- pragma export(set_param_lines(in, in, in, in, in, in, in, in, out),
 		"ML_BROWSE_set_param_lines").
 
 set_param_lines(P, B, A, F, Pr, V, Lines) -->
 	browser_info__set_param(P, B, A, F, Pr, V, lines(Lines)).
 
-:- pred set_param_format(bool, bool, bool, portray_format, browser_state,
-		browser_state).
-:- mode set_param_format(in, in, in, in, in, out) is det.
+:- pred set_param_format(bool::in, bool::in, bool::in, portray_format::in,
+		browser_state::in, browser_state::out) is det.
 :- pragma export(set_param_format(in, in, in, in, in, out),
 		"ML_BROWSE_set_param_format").
 
@@ -182,11 +182,11 @@
 		MaybeFormat = yes(Format)
 	;
 		MaybeFormat = no,
-		Override = Info ^ format,
+		MdbFormatOption = Info ^ format,
 		(
-			Override = yes(Format)
+			MdbFormatOption = yes(Format)
 		;
-			Override = no,
+			MdbFormatOption = no,
 			get_caller_params(Info ^ state, Caller, Params),
 			Format = Params ^ default_format
 		)
--- browser/parse.m.old	Fri Oct 27 14:22:03 2000
+++ browser/parse.m	Fri Oct 27 14:33:07 2000
@@ -98,9 +98,6 @@
 :- pred parse__read_command_external(command, io__state, io__state).
 :- mode parse__read_command_external(out, di, uo) is det.
 
-:- pred default_depth(int).
-:- mode default_depth(out) is det.
-
 %---------------------------------------------------------------------------%
 :- implementation.
 
@@ -275,16 +272,9 @@
 		Comm = print
 	;
 		Tok = (<),
-		( Toks = [] ->
-			default_depth(DefaultDepth),
-			Comm = set(depth(DefaultDepth))
-		;
-			Toks = [num(Depth)],
-			Comm = set(depth(Depth))
-		)
+		Toks = [num(Depth)],
+		Comm = set(depth(Depth))
 	).
-
-default_depth(3).
 
 :- pred parse_path(list(token), path).
 :- mode parse_path(in, out) is semidet.
--- doc/user_guide.texi.old	Fri Oct 27 02:10:53 2000
+++ doc/user_guide.texi	Fri Oct 27 02:30:43 2000
@@ -2129,11 +2129,14 @@
 @sp 1
 The browser also maintains separate configuration parameters
 for the three different output formats.
+This applies to all parameters except for the format itself.
 The options @samp{-f} or @samp{--flat}, @samp{-p} or @samp{--pretty},
 and @samp{-v} or @samp{--verbose}
 select which formats will be affected by the change.
 If none of these options is given,
 the default is to affect all formats.
+In the case that the format itself is being set,
+these options are ignored.
 @end table
 @sp 1
 @node Breakpoint commands
--- trace/mercury_trace_browse.c.old	Fri Oct 27 02:52:23 2000
+++ trace/mercury_trace_browse.c	Fri Oct 27 02:53:08 2000
@@ -146,7 +146,16 @@
 
 	MR_trace_browse_ensure_init();
 
-	if (streq(param, "depth") && MR_trace_is_number(value, &depth))
+	if (streq(param, "format") &&
+			MR_trace_is_portray_format(value, &new_format))
+	{
+		MR_TRACE_CALL_MERCURY(
+			ML_BROWSE_set_param_format(print, browse, print_all,
+				new_format, MR_trace_browser_state,
+				&MR_trace_browser_state);
+		);
+	}
+	else if (streq(param, "depth") && MR_trace_is_number(value, &depth))
 	{
 		MR_TRACE_CALL_MERCURY(
 			ML_BROWSE_set_param_depth(print, browse, print_all,
@@ -161,15 +170,6 @@
 			ML_BROWSE_set_param_size(print, browse, print_all,
 				flat, pretty, verbose, size,
 				MR_trace_browser_state,
-				&MR_trace_browser_state);
-		);
-	}
-	else if (streq(param, "format") &&
-			MR_trace_is_portray_format(value, &new_format))
-	{
-		MR_TRACE_CALL_MERCURY(
-			ML_BROWSE_set_param_format(print, browse, print_all,
-				new_format, MR_trace_browser_state,
 				&MR_trace_browser_state);
 		);
 	}
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------