[m-dev.] for review: new debugger command set (part 1 of 4)

[Fergus Henderson]

> >>> DOCUMENT NEW DEBUG COMMAND SET <<<
> 
> doc/user_guide.texi:
> 	Add a new section on the debugger. The description of the commands
> 	is complete, but some of the background sections, and the section
> 	about how to build debuggable executables, are not yet done.
> 
> 	Update the documentation of the tracing options.
> 
> doc/generate_mdb_doc:
> 	A new shell script that automatically converts some of the new
> 	sections of the user guide into the online documentation of the
> 	debugger.
> 
> doc/mdb_categories:
> 	The fixed initial part of the online documentation.
> 
> doc/mdbrc.in:
> 	A debugger command script that reads in the online documentation
> 	and then defines some standard aliases.
> 
> doc/Mmakefile:
> 	Add rules for creating mdb_doc, the file that is the online
> 	documentation of the debugger, and for installing it together
> 	with mdbrc.
> 
> Mmake.common.in:
> 	Define INSTALL_DOC_DIR for doc/Mmakefile.
> 
> configure.in:
> 	Define the variable that scripts/mdb.in and doc/mdbrc.in use to find
> 	the right files, and get configure to perform the substitutions.
> 
> configure.in:
> scripts/mdb:
> scripts/mdb.in:
> 	Replace mdb with mdb.in. Mdb is now created during configuration
> 	from mdb.in, filling in the name of the file that contains the default
> 	debugger initialization commands.
> 
> util/info_to_mdb.c:
> 	A program that does most of the work involved in automatically
> 	converting user guide sections into online documentation.
> 	(This couldn't easily be written in sh, because sh's read
> 	command has no notion of pushback.)
> 
> util/Mmakefile:
> 	Add info_to_mdb to the list of targets.
> 
> tools/bootcheck:
> 	Make sure that the tests in tests/debugger are executed with an
> 	initialization setup that is equivalent to what users will see
> 	by default.
...
> Index: doc/generate_mdb_command_list
> ===================================================================
> diff -N generate_mdb_command_list
> >>> DOCUMENT NEW DEBUG COMMAND SET <<<
> @@ -0,0 +1,17 @@
> +#!/bin/sh
> +

A comment here explaining what this file does would be helpful.

> +# the info menu items that get us to the chapter on debugger commands
> +cat mdb_categories > mdb_doc
> +
> +tmp="mdb_doc_tmp.$$"
> +trap '/bin/rm -f $tmp' 0 1 2 3 15
> +
> +info -f mercury_user_guide.info -o $tmp -n "Mercury debugger concepts"
> +../util/info_to_mdb concepts $tmp >> mdb_doc
> +
> +debug_cmd_path="debug debugger"
> +for section in forward backward browsing breakpoint parameter help exp developer misc
> +do
> +	info -f mercury_user_guide.info -o $tmp $debug_cmd_path $section
> +	../util/info_to_mdb $section $tmp >> mdb_doc
> +done
> Index: doc/mdb_categories
> ===================================================================
> diff -N mdb_categories
> >>> DOCUMENT NEW DEBUG COMMAND SET <<<
> --- /dev/null	Wed May 28 10:49:58 1997
> +++ mdb_categories	Mon Sep 21 18:49:25 1998
> @@ -0,0 +1,54 @@
> +document_category 100 concepts
> +concepts   - the concepts on which the Mercury debugger is based.

s/the/The/

Or alternatively, s/the concepts/Concepts/

> +end
> +document_category 200 forward
> +forward    - The commands that move execution forward.
> +             The forward commands are step, goto, finish, forward, and
> +             continue.

I think it would be clearer if the command names where in quotes
(`step', `goto', etc.).

Also I think it would read a little better if you do
	s/The commands/Commands/g
here and elsewhere.

> +end
> +document_category 300 backward
> +backward   - The commands that move execution backward.
> +             The backward command is retry.

The mixture of singular and plural here is awkward.
I suggest

backward   - Commands that move execution backward.
             The only command in this category is `retry'.

> +document_category 500 breakpoint
> +breakpoint - The commands that let users set and control breakpoints.
> +             The breakpoint commands are break, disable, enable,
> +             modules, procedures and register,

s/,/./

> +document_category 900 developer
> +developer  - The commands that should be of interest to developers only.

That could be misinterpreted, since any programmer might consider
themself a "developer".  Something like the following would be better:

   developer  - Commands that are intended to be of use only for the
   	        developers of the Mercury implementation.

> Index: user_guide.texi

The XXXs here mark some rather glaringly incomplete sections
of the documentation.  I don't think it would be appropriate
to commit it as is (remember that the documentation here goes
straight onto our WWW page).  You don't have to complete it
all now, but don't leave it with incomplete sentences --
it would be better to just comment those out (using `@c').
In some parts I think you also need a linking sentence or
two.

> +XXX to be filled in later
> +
> + at table @code
> + at item c at var{num}
> +The @var{num}'th conjunct of a conjunction.

You need a brief sentence explaining what this table is about.

> +
> +XXX
> +Most programs include 
> +
> +XXX
> +Programmers can control 
> +The fewer events are to be traced
> +the smaller the size of the executable

These should be commented out;
in their place you need a brief sentence introducing
the table below.

> + at table @var
> + at item trace level none
> +A procedure compiled with trace level none
> +will never generate any events.
> + at item trace level deep
> +A procedure compiled with trace level deep
> +will always generate all the events requested by the user.
> + at item trace level shallow
> +A procedure compiled with trace level shallow
> +will generate interface events
> +if it is called from a procedure compiled with trace level deep,
> +while it will generate no events

I suggest 
s/while it will generate no events/but it will not generate any events/

Also you should explain how/why programmers should use the different levels:
deep tracing for the potentially buggy code,
shallow tracing for code which is not expected to have bugs
(e.g. the standard library), but which is called from the 
potentially buggy code.

> +
> +c2init -i
> +

That should be XXXed, and then like the other XXXs, commented out.

> + at item strict commands
> +When a debugger command steps over some events
> +without user interaction at those events,
> +the *strictness* of the command controls whether

s/*strictness*/@emph{strictness}/

> + at item print level
> +When a debugger command steps over some events
> +without user interaction at those events,
> +the *print level* controls under what circumstances
> +the stepped over events will be printed.

Likewise.

> + at item scroll on
> +Turns on user control over the scrolling of sequences of event reports.
> +This means that every screenful of event reports
> +will be followed by a --more-- prompt.

s/--more--/@samp{--more--}/

> + at item scroll @var{size}
> +Sets the scroll window size to @var{size},
> +which tells scroll control to stop and print a --more-- prompt

Likewise.

> +after every @var{size - 1} events.
> +The default value of @var{size} is the value of the LINES environment variable,

s/LINES/@samp{LINES}/

> +++ info_to_mdb.c	Wed Sep  9 20:29:47 1998
...
> +#define	TRUE		1
> +#define	FALSE		0
> +#define	bool		char

Better to use `#include "mercury_std.h"'.

...
> +#define	concept_char(c)	(isupper(c) ? tolower(c) : (isspace(c) ? '_' : (c)))
> +#define	isalnumunder(c)	(isalnum(c) || (c) == '_')

You need to cast `c' to `unsigned char' here,
or (better) use `MR_isalnum', etc. from "mercury_std.h".

> +static bool
> +is_empty(char *line)

s/char */const char */

> +static bool
> +is_all_same_char(char *line, char what)
...
> +static bool
> +is_command(char *line, bool *is_concept)

Likewise.

> +{
> +	int	len;
> +
> +	len = strlen(line);
> +	if ((line[0] == '`') && (line[len-2] == '\'')) {
> +		*is_concept = FALSE;
> +		return TRUE;
> +	} else if (isupper(line[0]) && isupper(line[1])) {
> +		*is_concept = TRUE;
> +		return TRUE;
> +	} else {
> +		return FALSE;
> +	}
> +}

Here's some more occurrences of `isupper' that should be `MR_isupper'.

That's all of my comments on that change ("DOCUMENT NEW DEBUG COMMAND SET").
However, I haven't really gone through the changes to user_guide.texi
in great detail.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.