[m-rev.] For review: Added cdr command to interactive term browser

Julien Fischer juliensf at cs.mu.OZ.AU
Fri Oct 22 14:44:25 AEST 2004


On Fri, 22 Oct 2004, Ian MacLarty wrote:

> For review by anyone.
>
> Estimated hours taken: 1
> Branches: main
>
> Added a `cdr' command to the interactive term browser.  This repeatedly cd's
> into a path and is useful for accessing deep structures (like long lists).
>
> browser/browse.m
> 	Rewrote simplify_dirs predicate which removes redundant `..'
> 	directories.  The previous method was too inefficient and caused
> 	commands like `cdr 10000 ..' to take a long time.
> 	Added help text.
>
> browser/parse.m
> 	Added handler for `cdr' command which is just translated into a `cd'
> 	command with the path repeated the appropriate number of times.
>
> tests/debugger/browser_test.inp
> tests/debugger/browser_test.exp
> 	Added test for cdr command.
>
> Index: browser/browse.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/browser/browse.m,v
> retrieving revision 1.44
> diff -u -r1.44 browse.m
> --- browser/browse.m	9 Aug 2004 03:05:19 -0000	1.44
> +++ browser/browse.m	21 Oct 2004 23:58:26 -0000
> @@ -556,6 +556,7 @@
>  "\t[print|p|ls] [format_options] [path]\n",
>  "\t               -- print the specified subterm using the `browse' params\n",
>  "\tcd [path]      -- cd to the specified subterm (default is root)\n",
> +"\tcdr n path     -- repeatedly cd's into the path n times\n",

Is using an apostrophe to denote the plural of cd the correct thing to do
there?  (And in the log message as well).

>  "\tpwd            -- print the path to the current subterm\n",
>  "\tset [setting_options] var value\n",
>  "\t               -- set a parameter value\n",
> @@ -1471,34 +1472,35 @@
>
>  	% Remove "/dir/../" sequences from a list of directories to yield
>  	% a form that lacks ".." entries.
> -	% NB: This can be done more efficiently than simple iteration
> -	% to a limit.
> +	%
>  :- pred simplify_dirs(list(dir)::in, list(dir)::out) is det.
>
>  simplify_dirs(Dirs, SimpleDirs) :-
> -	util__limit(simplify, Dirs, SimpleDirs).
> +	list.reverse(Dirs, RevDirs),
> +	simplify_rev_dirs(RevDirs, 0, [], SimpleDirs).
>
> -	% If possible, remove a single occurence of
> -	% either:
> -	%	- "dir/../"
> -	% or:
> -	%	- "/.." (parent of root is root)
> +	% simplify_rev_dirs(RevDirs, N, SoFar, SimpleDirs).
> +	% Assumes a reverse list of directories and removes redundant `..'
> +	% entries by scanning from the bottom most directory to the top,
> +	% counting how many `..' occured (N) and removing entries accordingly.
> +	% SoFar accumulates the simplified dirs processed so far so we can be
> +	% tail recursive.
>  	%
> -:- pred simplify(list(dir)::in, list(dir)::out) is det.
> +:- pred simplify_rev_dirs(list(dir)::in, int::in, list(dir)::in,
> +	list(dir)::out) is det.
>
> -simplify([], []).
> -simplify([First | Rest], Simplified) :-
> -	( First = parent ->
> -		Simplified = Rest
> -	; Rest = [] ->
> -		Simplified = [First]
> -	; Rest = [parent | Tail] ->
> -		Simplified = Tail
> +simplify_rev_dirs([], _, SimpleDirs, SimpleDirs).
> +simplify_rev_dirs([Dir | Dirs], N, SoFar, SimpleDirs) :-
> +	( Dir = parent ->
> +		simplify_rev_dirs(Dirs, N+1, SoFar, SimpleDirs)
>  	;
> -		simplify(Rest, SimplifiedRest),
> -		Simplified = [First | SimplifiedRest]
> +		( N > 0 ->
> +			simplify_rev_dirs(Dirs, N-1, SoFar, SimpleDirs)
> +		;
> +			simplify_rev_dirs(Dirs, N, [Dir | SoFar], SimpleDirs)
> +		)
>  	).
> -
> +
>  :- func dir_to_string(dir) = string.
>
>  dir_to_string(parent) = "..".
> Index: browser/parse.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/browser/parse.m,v
> retrieving revision 1.19
> diff -u -r1.19 parse.m
> --- browser/parse.m	2 Sep 2004 04:03:50 -0000	1.19
> +++ browser/parse.m	21 Oct 2004 14:11:29 -0000
> @@ -347,6 +347,14 @@
>  			Command = cd(Path)
>  		)
>  	;
> +		CmdToken = name("cdr")
> +	->
> +		ArgTokens = [num(Repetitions) | TokenPath],
> +		list.duplicate(Repetitions, TokenPath, DupTokenPath),
> +		list.condense(DupTokenPath, RepeatedTokenPath),
> +		parse_path(RepeatedTokenPath, RepeatedPath),
> +		Command = cd(RepeatedPath)
> +	;
>  		CmdToken = name("pwd")
>  	->
>  		ArgTokens = [],
> Index: tests/debugger/browser_test.exp
> ===================================================================
> RCS file: /home/mercury1/repository/tests/debugger/browser_test.exp,v
> retrieving revision 1.17
> diff -u -r1.17 browser_test.exp
> --- tests/debugger/browser_test.exp	13 May 2004 08:50:31 -0000	1.17
> +++ tests/debugger/browser_test.exp	21 Oct 2004 23:45:04 -0000
> @@ -94,6 +94,15 @@
>    big(big(small, 1, small), 2, small),
>    3,
>    big(big(small, 4, big(small, 5, small)), 6, small))
> +browser> cdr 3 1
> +browser> ls
> +small
> +browser> cdr 3 ../1/..
> +browser> ls
> +big(
> +  big(big(small, 1, small), 2, small),
> +  3,
> +  big(big(small, 4, big(small, 5, small)), 6, small))
>  browser> quit
>  mdb> set -A -f depth 1
>  mdb> print *

I think you should add a test where you try to cdr into
an invalid location as well.

> Index: tests/debugger/browser_test.inp
> ===================================================================
> RCS file: /home/mercury1/repository/tests/debugger/browser_test.inp,v
> retrieving revision 1.9
> diff -u -r1.9 browser_test.inp
> --- tests/debugger/browser_test.inp	13 May 2004 08:50:31 -0000	1.9
> +++ tests/debugger/browser_test.inp	21 Oct 2004 14:17:02 -0000
> @@ -30,6 +30,10 @@
>  ls
>  cd
>  ls
> +cdr 3 1
> +ls
> +cdr 3 ../1/..
> +ls
>  quit
>  set -A -f depth 1
>  print *

Otherwise that looks fine.

Cheers,
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