[m-rev.] for review: source-linking for mdb

Fergus Henderson fjh at cs.mu.OZ.AU
Sat Oct 27 14:27:46 AEST 2001


On 26-Oct-2001, Mark Brown <dougl at cs.mu.OZ.AU> wrote:
> New commands added to mdb are 'source_window', which
> opens a source-linked window if there is not one already open, and
> 'source_window_close' which shuts any open source window.

Those command names are long and ugly,
which is not good for interactive use.
Something more concise would be much better.
E.g. how about `edit' and `edit -c' (or `edit --close')?

Also IMHO there should be a single-character alias for the
first command, e.g. `alias e edit'.

> +		}
> +		else if (word_count != 1)
> +		{

The } should be on the same line as the else
and the { should be on the same line as the if.

Likewise through-out this diff.

> +static const char *
> +MR_trace_new_source_window(const char *window_cmd, const char *server_cmd,
> +		const char *server_name, int timeout, bool force)

It would help to document what that function does.

> +static	void
> +MR_trace_maybe_sync_source_window(MR_Event_Info *event_info)

Likewise.

> +	const MR_Label_Layout	*parent;
> +	const char		*filename;
> +	int			lineno;
> +	const char		*problem; /* not used */
> +	MR_Word			*base_sp, *base_curfr;
> +	const char		*msg;
> +
> +	if (MR_trace_source_server.server_name != NULL) {
> +		lineno = 0;
> +		filename = "";
> +
> +		/*
> +		** At interface ports we use the parent context if we can.
> +		** Otherwise, we use the current context.
> +		*/
> +		if (MR_port_is_interface(event_info->MR_trace_port)) {
> +			base_sp = MR_saved_sp(event_info->MR_saved_regs);
> +			base_curfr = MR_saved_curfr(event_info->MR_saved_regs);
> +			parent = MR_find_nth_ancestor(event_info->MR_event_sll,
> +				1, &base_sp, &base_curfr, &problem);
> +			if (parent != NULL) {
> +				(void) MR_find_context(parent, &filename,
> +					&lineno);
> +			}
> +		}
> +
> +		if (filename[0] == '\000') {

s/000/0/

> +	/*
> +	** Start the server in read-only mode, to discourage the user
> +	** from trying to edit the source.
> +	*/
> +	sprintf(system_call, "%s %s -R --servername \"%s\" &",
> +			real_window_cmd, real_server_cmd, name);

I think it might be nicer to make `-R' part of the default
server name ("vim -R"), so that the user can turn that off
if they want.

> +	/*
> +	** We first send "Ctrl-\ Ctrl-N", which guarantees we get back to
> +	** "normal" mode without beeping,

How is that supposed to work?

It doesn't seem to work when I tried testing it.
E.g. from insert mode, Ctrl-\ Ctrl-N had no effect.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  | "... it seems to me that 15 years of
The University of Melbourne         | email is plenty for one lifetime."
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- Prof. Donald E. Knuth
--------------------------------------------------------------------------
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