[m-rev.] split-screen source linking

[Mark Brown]

On 05-Nov-2001, Ralph Becket <rafe at cs.mu.OZ.AU> wrote:
> 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?

Yes, I should give some more detail here.  I'll make that part of the
change after we have settled on what the appearance should be.  (See
below.)

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

Ok, I've #define'd each of the control strings to give them meaningful
names.  This is probably a better way of factoring the code than the
focus_on_* functions, because consecutive strings can easily be composed
within a single send, but calling focus_on_* followed by some other
command mat require two or more sends.  We will want to minimise the
number of sends wherever possible, of course.

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

My view is that the "action" is at the call site, rather than the
definition.  The reason for this is that to understand how execution
proceeds through a procedure body, we want to look at the internal
events of that procedure in conjunction with the interface events of the
*children* of that procedure.  So we want to consider each interface
event in its role as child of its caller, rather than its dual role as
head of the callee.  This is why, in single-window mode, we show the
caller rather than callee at interface events.

At interface events in split-window mode, the upper window shows the
caller and the lower window shows the callee, which is what your original
suggestion was (although I consider the action to be in the top window).
Internal events, however, refer to a compound goal rather than a call
site; as such, there is no caller/callee, just a single location in a
source file.  Clearly we should display this location in the "action"
window, but what should the other window show?

I have chosen to display the internal event in the top window, and leave
the other window unchanged, which means that it will still show the body
of the last call made.  I agree that this information is, in a sense, out
of date.  But we don't really have any useful information to put there,
and I think if there is a change in that window at all, then the user will
see a noticeable flicker which is likely to draw their eyes down to that
window.  This may, in effect, mislead them into thinking that there is
useful information in that window.

Is there something specific you would rather see displayed in the
non-action window, at internal events?

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

Ah, excellent.  I was hoping there would be a better way to send the
strings than as raw control characters.  Aside from being more
maintainable, it also means that the strings can be echoed in verbose
mode without screwing up the terminal, so I have made that change also.

> 
> > +			/* 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'.

Ok.

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

Probably the source-linking feature as a whole should be mentioned.
I'll do that.

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

I won't check it in yet, since I haven't yet updated the documentation
as you suggested.  Below is the relative diff of the changes so far.

Cheers,
Mark.

diff -u trace/mercury_trace_source.c trace/mercury_trace_source.c
--- trace/mercury_trace_source.c
+++ trace/mercury_trace_source.c
@@ -36,12 +36,33 @@
 /*
 ** When sent to a vim server, this string puts vim into "normal" mode.
 ** This has much the same effect as ESC, except that there is no bell if
-** already in normal mode.
+** already in normal mode.  We send this before most of the other
+** commands to ensure that they are interpreted correctly.
 **
 ** See the vim help page for ctrl-\_ctrl-n (type ':he ** ctrl-\\_ctrl-n'
 ** in vim).
 */
-#define MR_SOURCE_SERVER_RESET_STRING           "\034\016"
+#define MR_SOURCE_SERVER_RESET_STRING           "<C-\\><C-N>"
+
+/*
+** These vim commands split the screen, move to the bottom window, and
+** move up one window, respectively.
+*/
+#define MR_SOURCE_SERVER_SPLIT_STRING		"<C-W>s"
+#define MR_SOURCE_SERVER_BOTTOM_STRING		"<C-W>b"
+#define MR_SOURCE_SERVER_UP_STRING		"<C-W>k"
+
+/*
+** This vim command centres the current window on the cursor.
+*/
+#define MR_SOURCE_SERVER_CENTRE_STRING		"z."
+
+/*
+** This is the command we use to quit the server.  We have to close *all*
+** windows, just in case we are in split screen mode.  This won't quit if
+** the user has modified any file, which is just as well.
+*/
+#define MR_SOURCE_SERVER_QUIT_STRING		":qall<CR>"
 
 /*
 ** The name used for the server will start with this basename.
@@ -278,10 +299,12 @@
 	}
 
 	if (server->split) {
-		/* Split the window with "^Ws". */
+		/* Split the window. */
 		status = MR_trace_source_send(real_server_cmd,
 				server->server_name,
-				MR_SOURCE_SERVER_RESET_STRING "\027s", FALSE);
+				MR_SOURCE_SERVER_RESET_STRING
+				MR_SOURCE_SERVER_SPLIT_STRING,
+				verbose);
 		if (status != 0) {
 			server->split = FALSE;
 			return "warning: unable to split source window";
@@ -374,11 +397,12 @@
 		*/
 
 		if (have_parent && have_current) {
-			/* Move to the bottom window with "^Wb". */
+			/* Move to the bottom window. */
 			status = MR_trace_source_send(real_server_cmd,
 					server->server_name,
-					MR_SOURCE_SERVER_RESET_STRING "\027b",
-					FALSE);
+					MR_SOURCE_SERVER_RESET_STRING
+					MR_SOURCE_SERVER_BOTTOM_STRING,
+					verbose);
 			if (status != 0) {
 				return "warning: source synchronisation failed";
 			}
@@ -390,23 +414,25 @@
 				return msg;
 			}
 
-			/* Move up one window with "^Wk". */
+			/* Move up one window. */
 			status = MR_trace_source_send(real_server_cmd,
 					server->server_name,
-					MR_SOURCE_SERVER_RESET_STRING "\027k",
-					FALSE);
+					MR_SOURCE_SERVER_RESET_STRING
+					MR_SOURCE_SERVER_UP_STRING,
+					verbose);
 			if (status != 0) {
 				return "warning: source synchronisation failed";
 			}
 		} else {
 			/*
-			** Move to the second from bottom window with
-			** "^Wb^Wk".
+			** Move to the second from bottom window.
 			*/
 			status = MR_trace_source_send(real_server_cmd,
 					server->server_name,
 					MR_SOURCE_SERVER_RESET_STRING
-					"\027b\027k", FALSE);
+					MR_SOURCE_SERVER_BOTTOM_STRING
+					MR_SOURCE_SERVER_UP_STRING,
+					verbose);
 			if (status != 0) {
 				return "warning: source synchronisation failed";
 			}
@@ -458,12 +484,10 @@
 	** Center the current line in the vim window.  We need to put
 	** the server in normal mode, just in case the user has changed
 	** mode since the previous command was sent.
-	**
-	** Avoid echoing the command even if --verbose is set, because
-	** the control characters may interfere with the terminal.
 	*/
 	status = MR_trace_source_send(server_cmd, server_name,
-			MR_SOURCE_SERVER_RESET_STRING "z.", FALSE);
+			MR_SOURCE_SERVER_RESET_STRING
+			MR_SOURCE_SERVER_CENTRE_STRING, verbose);
 	if (status != 0) {
 		return "warning: source synchronisation failed";
 	}
@@ -489,17 +513,9 @@
 		return msg;
 	}
 
-	/*
-	** We first send "Ctrl-\ Ctrl-N", which guarantees we get back to
-	** "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 ":qall\n", FALSE);
+			MR_SOURCE_SERVER_RESET_STRING
+			MR_SOURCE_SERVER_QUIT_STRING, verbose);
 
 #if 0
 	/*
only in patch2:
--- NEWS	4 Nov 2001 09:06:33 -0000	1.224
+++ NEWS	5 Nov 2001 23:03:49 -0000
@@ -106,6 +106,11 @@
   have functions.
 
 Changes to the Mercury implementation:
+
+* We've added a 'view' command to `mdb', which opens a `vim' window and
+  in it displays the current source location, updated at each event.  This
+  requires X11 and a version of `vim' with the `clientserver' feature
+  enabled.
   
 * The Mercury compiler can now perform smart recompilation, enabled by the
   `--smart-recompilation' option. With smart recompilation, when the
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------