[m-dev.] for review: enhancements to mtags script

[Fergus Henderson]

On 06-Jul-1998, David Matthew Overton <dmo at cs.mu.OZ.AU> wrote:
> 
> Enhance the mtags script to produce tags for typeclasses and to
> support some new features of Vim and Elvis.

Thanks, David.  Those changes look good.

I have a few suggestions.

> scripts/mtags:
>         Add support for producing tags for typeclass, instance and
> 	method declarations.
> 
> 	Add new options `--elvis' and `--ext' which work as follows:
> 
> 	`mtags --elvis':
> 		Works the same as `mtags --vim'.  This produces tags
> 		that will work in versions of Vim before 5.0 and
> 		versions of Elvis before 2.1.
> 
> 	`mtags --ext':
> 		Produce tags in the extended format used by Vim 5.0+.
> 		Duplicate tags are not removed from the tags file.
> 		Extra attributes are added to each tag to say whether
> 		it is in the implementation or interface of the source
> 		file and to describe the kind of tag.  Tag kinds used
> 		are:
> 		`pred':	for predicate declarations
> 		`func': for function declarations
> 		`type': for type definitions
> 		`cons': for type constructors
> 		`inst': for inst definitions
> 		`mode': for mode definitions
> 		`tc':	for typeclass declarations
> 		`tci':	for typeclass instance declarations
> 		`tcm':	for typeclass methods
> 		`tcim':	for typeclass instance methods
> 
> 		(Vim assumes that the `kind' attribute has at most 4
> 		characters.)
> 
> 		Current limitation: assumes each method declaration
> 		start on a new line.
> 
> 	`mtags --ext --elvis':
> 		Similar to `mtags --ext', but produces a slightly
> 		different format that works with Elvis 2.1+.

This documentation should go in some place other than just the log
message.  Probably the best thing to do is to add support for a
`--help' option that displays this documentation, and to
add `mtags' to the list of manpages in doc/Mmakefile; the man page
will then be created automatically from the output of `--help'.
(Make sure you check that the man page looks OK.)

Previously the options were simple enough that there wasn't really
much need for documentation.

> diff -u -r1.16 mtags
> --- 1.16	1997/08/27 14:23:23
> +++ mtags	1998/07/06 01:40:09
> @@ -23,6 +23,8 @@
>  $warnings = 0;
>  $emacs = 0;
>  $vim = 0;
> +$ext = 0;
> +$elvis = 0;
>  
>  OPTION:
>  while ($#ARGV >= 0 && $ARGV[0] =~ /^-/) {
> @@ -33,12 +35,25 @@
>  	}
>  	if ($ARGV[0] eq "--vim") {
>  		$vim = 1;
> +		$elvis = 0;
>  		shift(ARGV);
>  		next OPTION;
>  	}
> +	if ($ARGV[0] eq "--ext") {
> +		$ext = 1;
> +		shift(ARGV);
> +		next OPTION;
> +	}
> +	if ($ARGV[0] eq "--elvis") {
> +		$elvis = 1;
> +		$vim = 0;
> +		shift(ARGV);
> +		next OPTION;
> +	}
>  }

I just noticed a bug: there should be a call to "die" before the
closing } there, otherwise it just goes into an infinite loop if you
give it an unrecognized option.  Also it should treat "--" as the end
of all options.  You might as well fix those bugs while you're at it
(since if anyone else does it, it will probably result in lots of CVS
conflicts).

I think the following code

	if ($ARGV[0] eq "-h" || $ARGV[0] eq "--help") {
		print "$help";
		exit(0);
	}
	if ($ARGV[0] eq "--") {
		shift(ARGV);
		break;
	}
	die "mtags: unrecognized option \`$ARGV[0]'\n" .
	    "Use \`mtags --help' for help.\n";

should do the trick, presuming you set $help to something appropriate
first (see below).   But I haven't tested that code.

> -die "Usage: mtags [--vim] [-e] [--emacs] file1.m ...\n" if $#ARGV < 0;
> +die "Usage: mtags [--vim | --elvis] [--ext] [-e] [--emacs] file1.m ...\n" 
> +	if $#ARGV < 0;

This should change to something like

	die $usage if $#ARGV < 0;

where $usage (and $help) are set at the top of the file,
with something along these lines:

	$usage = "\
	Usage: mtags [options] file1.m ...
	Use \`mtags --help' for help.";

	$help = "\
	Usage: mtags [options] file1.m ...
	Description:
		...
	Options:
		--vim
			...
		--elvis
			...
		...
	";

> +		} else {
> +		    # Works with vim or vi (vi ignores anything after the `;"').
> +		    printf out "%s\t%s\t/^%s\$/;-;/%s/;\"\t%s%s\n",
> +			$name, $file, $match_line, $name, $kind, $static;

The comment here is not correct.
vi does not ignore anything after the ";".
Instead, vi allows the thing after the 2nd tab to be an arbitrary ex command,
or a sequence of multiple `ex' commands, seperated by semicolons.
So the above command does a search for a line exactly matching $match_line,
goes back one line (`-'), and then does a search for $name.
This leaves the cursor on the matching line but puts $name in the
string search buffer.

I am guilty of not documenting this clearly, of course, but
I did explain the rationale for it in my log message:

        Enhance the vi command we output to find the tag: it now puts
        the tag in the string search buffer so that you can use the `n'
        (next) command in vi to search for the tag, in case the
        tag lookup gets you the pred declaration when you really
        wanted the pred body.

If you could replace the incorrect comment with a correct one
incorporating some of the explanation above, that would be great.

> +	# Shorten $kind for typeclass and instance so they display better in
> +	# vim which assumes the kind attribute has at most 4 chars.
> +	if ($kind eq "typeclass") {$kind = "tc";}
> +	if ($kind eq "instance") {$kind = "tci";}

s/{/{ /
s/}/ }/

This change should get another round of reviewing, so can you please
send us another diff when you've addressed those comments?

Thanks,
	Fergus.

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