[m-rev.] split-screen source linking

Ralph Becket rafe at cs.mu.OZ.AU
Mon Nov 5 11:21:14 AEDT 2001


Mark Brown, Friday,  2 November 2001:
> 
> Index: doc/user_guide.texi
> ===================================================================
> + at sp 1
> +The option @samp{-2} (or @samp{--split-screen})
> +starts the vim server with two windows.
> +This allows both the callee as well as the caller
> +to be displayed at interface events.

Perhaps you should mention which appears in which window?

> Index: trace/mercury_trace_internal.c
> ===================================================================
>  
>  /*
> +** Tell the server to jump to the given file and line.  If the server has
> +** multiple windows open, use the current window.  If successful, returns
> +** NULL, otherwise returns an error message.
> +*/

It might be more robust if you had a couple of functions like
"focus_on_caller_window" and "focus_on_callee_window" and used
those terms instead.

In fact, it might be an idea to factor out all the different
command strings you want to send in order to completely
separate out the control logic from the details of talking to
vim.

> +	if (server->split) {
> +		/*
> +		** When in split mode, we use the bottom two windows on
> +		** the vim server.  The window second from bottom (which
> +		** will usually be the top window) displays what would be
> +		** shown in non-split mode.
> +		**
> +		** If there is no parent context (e.g. at internal events)
> +		** we leave the bottom window alone.
> +		**
> +		** If we have a parent context it will be displayed in the
> +		** window second from bottom.  So in this case we show the
> +		** current context in the bottom window.
> +		*/

This seems like it's going to look odd to me.  My original idea was
that all the "action" would occur in the lower window, with the top
window showing the return location.  What you describe here sounds
like the lower (callee) window can occasionally contain misleading
(out of date) information.

> +
> +		if (have_parent && have_current) {
> +			/* Move to the bottom window with "^Wb". */
> +			status = MR_trace_source_send(real_server_cmd,
> +					server->server_name,
> +					MR_SOURCE_SERVER_RESET_STRING "\027b",

The vim --remote-send command recognises vim-style control sequences, so
the above could be written 

					MR_SOURCE_SERVER_RESET_STRING "<C-W>b",

which is more maintainable.

> +			/* Move up one window with "^Wk". */
> +			status = MR_trace_source_send(real_server_cmd,
> +					server->server_name,
> +					MR_SOURCE_SERVER_RESET_STRING "\027k",

Ditto.

> +			/*
> +			** Move to the second from bottom window with
> +			** "^Wb^Wk".
> +			*/
> +			status = MR_trace_source_send(real_server_cmd,
> +					server->server_name,
> +					MR_SOURCE_SERVER_RESET_STRING
> +					"\027b\027k", FALSE);

Ditto.

>  	/*
>  	** We first send "Ctrl-\ Ctrl-N", which guarantees we get back to
> -	** "normal" mode without beeping, followed by ":q\n" which should
> -	** quit.  This won't quit if the user has modified the file, which
> -	** is just as well.
> +	** "normal" mode without beeping, followed by ":qall\n" which should
> +	** quit all windows.  This won't quit if the user has modified any
> +	** file, which is just as well.
>  	**
>  	** Avoid echoing the command even if --verbose is set, because
>  	** the control characters may interfere with the terminal.
>  	*/
>  	MR_trace_source_send(real_server_cmd, server->server_name,
> -			MR_SOURCE_SERVER_RESET_STRING ":q\n", FALSE);
> +			MR_SOURCE_SERVER_RESET_STRING ":qall\n", FALSE);

For consistency, you could use <CR> rather than '\n'.



Shouldn't this be mentioned in the NEWS file as well?



I'm happy for this to be checked in now, although I would like to see
some of the vim-specific stuff refactored some time.

Cheers,

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