[comp.mail.mush] mushtool bugs and patches

del@thrush.semi.harris-atd.com (Don Lewis) (11/21/89)

I've been using mush 7.0.0.0 for a week or so, primarily in tool mode under
SunOS 4.0.1.  I have found a few problems and thought I'd share my patches
with you.

The first problem is that the pop-up windows don't get destroyed when you
are done with them causing mushtool to eventually to run out of file
descriptors and core dump.  The FRAME_NO_CONFIRM is optional, but I think
it's less of a hassle to use this way.  Maybe it should be a user settable
option.


*** ORIGmisc.c	Thu Sep  7 11:26:16 1989
--- misc.c	Mon Nov 13 20:58:07 1989
***************
*** 476,481 ****
--- 476,483 ----
  		text_frame = window_create(tool, FRAME,
  		    FRAME_SHOW_LABEL,	TRUE,
  		    FRAME_LABEL,	more_prompt,
+ 		    FRAME_DONE_PROC,	window_destroy,
+ 		    FRAME_NO_CONFIRM,	TRUE,
  		    WIN_HEIGHT,		l_height(curfont) *
  				    (crt_win? max(atoi(crt_win), 10) : 30),
  		    NULL);



The second problem causes the "Next" button in the main panel to pop up
a window to browse the message intermittently.  BTW, is there any good
reason that the buttons were implemented as choice items with display
of the choices turned of versus button items with menus?


*** ORIGdoproc.c	Sat Sep 16 17:42:57 1989
--- doproc.c	Mon Nov 13 21:53:50 1989
***************
*** 69,74 ****
--- 69,76 ----
      /* check "event" in case we were called from select.c
       * in which case event would be NULL
       */
+     if (event && event_id(event) == MS_LEFT)
+ 	value = 0;
      if (event && event_id(event) == MS_RIGHT &&
  	item && (item == read_item && value > 1 ||
  	(item == sub_hdr_item[0] || item == sub_hdr_item[1])))


The third problem causes the "f" or "r" flags to be set on a piece of mail
if you start to reply to or forward this piece of mail and then abort before
actually sending the the mail.  This patch is a bit ugly.  It defers setting
these flags until after the mail is send.  Any ideas on better ways of doing
this?


*** ORIGmail.c	Fri Sep 15 17:11:28 1989
--- mail.c	Mon Nov 13 16:55:00 1989
***************
*** 36,41 ****
--- 36,43 ----
  static int send_it();
  static long add_headers();
  static jmp_buf cntrl_c_buf;
+ static int is_responding;
+ static int forwarded_message;
  FILE *ed_fp;
  char *edfile, *mktemp();
  
***************
*** 155,161 ****
      if (do_set(set_options, "autosign"))
  	turnon(flgs, SIGN);
      *in_reply_to = *To = *Subject = *Cc = *Bcc = 0;
!     if (lower(firstchar) == 'r') {
  	char *old_fmt = hdr_format, *pcc = NULL;
  	to = To, cc = Cc;
  	/*
--- 157,163 ----
      if (do_set(set_options, "autosign"))
  	turnon(flgs, SIGN);
      *in_reply_to = *To = *Subject = *Cc = *Bcc = 0;
!     if (is_responding = (lower(firstchar) == 'r')) {
  	char *old_fmt = hdr_format, *pcc = NULL;
  	to = To, cc = Cc;
  	/*
***************
*** 390,396 ****
  		set_msg_bit(fwd, i);
  		if (start_file(fwd) < 0)
  		    return -1;
! 		turnon(msg[i].m_flags, FORWARD);
  		clear_msg_list(fwd);
  	    }
      } else
--- 392,401 ----
  		set_msg_bit(fwd, i);
  		if (start_file(fwd) < 0)
  		    return -1;
! 		if (!istool)
! 		    turnon(msg[i].m_flags, FORWARD);
! 		else
! 		    forwarded_message=i;
  		clear_msg_list(fwd);
  	    }
      } else
***************
*** 1053,1058 ****
--- 1058,1065 ----
      /* forwarded mail has no additional personalized text */
      if (ison(flags, FORWARD)) {
  	(void) send_it();
+ 	if (istool && isoff(flags, EDIT))
+ 	    turnon(msg[forwarded_message].m_flags, FORWARD);
  	turnoff(glob_flags, IS_GETTING);
  	return 1;
      }
***************
*** 1102,1107 ****
--- 1109,1116 ----
      if (!send_it())
  	return 0;
      turnoff(glob_flags, IS_GETTING);
+     if (is_responding && istool)
+ 	set_reply_bits();
      return 1;
  }
  


*** ORIGcommands.c	Fri Sep 15 15:56:51 1989
--- commands.c	Mon Nov 13 16:38:56 1989
***************
*** 435,448 ****
      return 0;
  }
  
  respond(n, argv, list)
  register int n;  /* no use for argc, so use its address space for a variable */
  register char **argv, *list;
  {
      register char *cmd = *argv;
!     char list1[MAXMSGS_BITS];
!     int cur_msg = current_msg, save_cnt = msg_cnt;
  
      if (*++argv && !strcmp(*argv, "-?"))
  	return help(0, "respond", cmd_help);
      if ((n = get_msg_list(argv, list)) == -1)
--- 435,468 ----
      return 0;
  }
  
+ static char list1[MAXMSGS_BITS];
+ static int save_cnt;
+ 
+ set_reply_bits()
+ {
+     int n;
+     /* New mail may have arrived during do_mail(), which will change
+      * the msg_cnt.  Use the old count when examining the list of bits
+      * to set the replied flag, or the wrong messages can be marked.
+      */
+     for (n = 0; n < save_cnt; n++)
+ 	if (msg_bit(list1, n)) {
+ 	    /* set_isread(n); */
+ 	    set_replied(n); /* only if mail got delivered */
+ 	}
+     if (istool)
+ 	(void) do_hdrs(0, DUBL_NULL, NULL);
+ }
+ 
  respond(n, argv, list)
  register int n;  /* no use for argc, so use its address space for a variable */
  register char **argv, *list;
  {
      register char *cmd = *argv;
!     int cur_msg = current_msg;
  
+     save_cnt = msg_cnt;
+ 
      if (*++argv && !strcmp(*argv, "-?"))
  	return help(0, "respond", cmd_help);
      if ((n = get_msg_list(argv, list)) == -1)
***************
*** 467,483 ****
      }
      if (do_mail(1 /* ignored */, argv, list) == -1)
  	return -1;
!     /* New mail may have arrived during do_mail(), which will change
!      * the msg_cnt.  Use the old count when examining the list of bits
!      * to set the replied flag, or the wrong messages can be marked.
!      */
!     for (n = 0; n < save_cnt; n++)
! 	if (msg_bit(list1, n)) {
! 	    /* set_isread(n); */
! 	    set_replied(n); /* only if mail got delivered */
! 	}
!     if (istool)
! 	(void) do_hdrs(0, DUBL_NULL, NULL);
      /* copy the specified list back into msg_list */
      bitput(list1, list, MAXMSGS, =);
      return 0;
--- 487,496 ----
      }
      if (do_mail(1 /* ignored */, argv, list) == -1)
  	return -1;
! 
!     if (!istool)
! 	set_reply_bits();
! 
      /* copy the specified list back into msg_list */
      bitput(list1, list, MAXMSGS, =);
      return 0;


I have also noticed some problems with the function keys.

	The function keys only work in certain windows.

	The Sunview open/close and front/back keys also get
	interpreted as function keys.

	The fkey command doesn't seem to know about the built
	in key definitions.
	
	Some of the built in key bindings are sort of bogus
	in these modern times.

	What happened to the facility for setting the fkeys
	in tool mode (the pop up keyboard picture) that was
	in 6.5.6?

Is anyone out there working on these?  I have a few ideas, but
if only I had the time....
--
Don "Truck" Lewis                                     Harris Semiconductor
Internet (if you're lucky): del@semi.harris-atd.com   PO Box 883   MS 62A-028
Internet (if not): del%thrush@trantor.harris-atd.com  Melbourne, FL  32901
UUCP (works): rutgers!soleil!thrush!del               Phone: (407) 729-5205

schaefer@ogccse.ogc.edu (Barton E. Schaefer) (11/22/89)

In article <1989Nov21.055514.6990@semi.harris-atd.com> del@thrush.semi.harris-atd.com (Don Lewis) writes:
} I've been using mush 7.0.0.0 for a week or so, primarily in tool mode under
} SunOS 4.0.1.  I have found a few problems and thought I'd share my patches
} with you.

Thanks for the suggestions.

} The first problem is that the pop-up windows don't get destroyed when you
} are done with them causing mushtool to eventually to run out of file
} descriptors and core dump.

This has already been fixed in the semi-completed patches that are sitting
around waiting for Dan to get done with his book.

} The second problem causes the "Next" button in the main panel to pop up
} a window to browse the message intermittently.

I *think* this one has been fixed as well (by virtue of drastic changes
in much of the toolmode code, i.e., I think that whole section is gone).
However, I'll hang onto the patch anyway.

} BTW, is there any good
} reason that the buttons were implemented as choice items with display
} of the choices turned of versus button items with menus?

Because 7.0.0.0 is still pretty much a direct translation from SunWindows.
More work has been done since then to use SunView "right".

} The third problem causes the "f" or "r" flags to be set on a piece of mail
} if you start to reply to or forward this piece of mail and then abort before
} actually sending the the mail.  This patch is a bit ugly.

All the code for sending mail in 7.0.0.0 toolmode is more than a bit ugly,
and has completely changed.  I *know* this patch won't work on the latest
mail.c, but I'll keep the bug report in case it's still a problem.

} I have also noticed some problems with the function keys.
} 
} 	The function keys only work in certain windows.

The function keys are going away altogether as I understand it.
Part of the reason:

} 	The Sunview open/close and front/back keys also get
} 	interpreted as function keys.

This is a SunView problem -- if (older) mushes had their way, the
open/close and front/back functions would be disabled and the keys used
for mush's purposes, but ever since the window managers have assigned
meanings to those keys there has been no way to avoid having *both* the
window manager event and the program event happen when the key is hit.
Another reason for getting rid of the fkeys:

} 	Some of the built in key bindings are sort of bogus
} 	in these modern times.

And fkeys going away also explains:

} 	What happened to the facility for setting the fkeys
} 	in tool mode (the pop up keyboard picture) that was
} 	in 6.5.6?
-- 
Bart Schaefer          "Sometimes I think the surest sign that intelligent life
                        exists elsewhere in the universe is that none of it has
schaefer@cse.ogi.edu        tried to contact us."                     -- Calvin
(used to be cse.ogc.edu)

argv%turnpike@Sun.COM (Dan Heller) (11/22/89)

> } The third problem causes the "f" or "r" flags to be set on a piece of mail
> } if you start to reply to or forward this piece of mail and then abort before
> } actually sending the the mail.  This patch is a bit ugly.
> All the code for sending mail in 7.0.0.0 toolmode is more than a bit ugly,
> and has completely changed.  I *know* this patch won't work on the latest
> mail.c, but I'll keep the bug report in case it's still a problem.

Currently, this won't work anyway because of the event-driven nature of
the toolmode.  For example, you can reply to a message and then resort
everything using the "sort" menu.  If you successfully send the message,
you could potentially mark the wrong message as being replied to.

Further, the is going to be a facility for multiple compositions.
This means that you could reply to the same message in different
windows, abort one message, but complete the other.  Again, the
event-driven nature interferes.  Someday, this will be fixed correctly.

> } I have also noticed some problems with the function keys.
> } 	The function keys only work in certain windows.
> The function keys are going away altogether as I understand it.
All of the reasons bart mentioned for function keys going away are true.
But the function keys profice a nice way to "accelerate" actions within
the window.  As a replacement (someday), there is going to be a "command"
window similar to dbxtool, maybe.  That is, you will be able to enter
mush commands directly from a special window or prompt.

dan

del@thrush.semi.harris-atd.com (Don Lewis) (11/22/89)

In article <128221@sun.Eng.Sun.COM> argv@sun.UUCP (Dan Heller) writes:
>> } The third problem causes the "f" or "r" flags to be set on a piece of mail
>> } if you start to reply to or forward this piece of mail and then abort before
>> } actually sending the the mail.  This patch is a bit ugly.
>> All the code for sending mail in 7.0.0.0 toolmode is more than a bit ugly,
>> and has completely changed.  I *know* this patch won't work on the latest
>> mail.c, but I'll keep the bug report in case it's still a problem.
>
>Currently, this won't work anyway because of the event-driven nature of
>the toolmode.  For example, you can reply to a message and then resort
>everything using the "sort" menu.  If you successfully send the message,
>you could potentially mark the wrong message as being replied to.
>
>Further, the is going to be a facility for multiple compositions.
>This means that you could reply to the same message in different
>windows, abort one message, but complete the other.  Again, the
>event-driven nature interferes.  Someday, this will be fixed correctly.
>
This is worse than I thought.  Sigh.

>> } I have also noticed some problems with the function keys.
>> } 	The function keys only work in certain windows.
>> The function keys are going away altogether as I understand it.
>All of the reasons bart mentioned for function keys going away are true.
>But the function keys profice a nice way to "accelerate" actions within
>the window.  As a replacement (someday), there is going to be a "command"
>window similar to dbxtool, maybe.  That is, you will be able to enter
>mush commands directly from a special window or prompt.
>

But in the mean time, the function keys are the only way to do useful
operations that are not possible given the buttons and menus.  I use
one of the function keys to do "mail -f".  So see below for a throwaway
patch for this until we get the real thing.


This is a simple patch to make the Open, Front, and Props function keys not
do any bogus mushtool function key actions.  It's not as easy is totally
disabling the function key code, but at least the other function keys are
still usable (at least in the subwindows that listen to them).


*** ORIGselect.c	Wed Sep  6 12:36:25 1989
--- select.c	Tue Nov 21 23:39:45 1989
***************
*** 54,59 ****
--- 54,60 ----
      struct rect rect;
      static char 	lastchar;
      static int 		line, count;
+     Inputmask		im;
  
      if (ID == WIN_REPAINT || ID == WIN_RESIZE) {
  	if (exec_pid)
***************
*** 61,71 ****
  	msg_rect = *((Rect *) window_get(msg_sw, WIN_RECT));
  	crt = msg_rect.r_height / l_height(curfont);
      }
!     else if (ID >= KEY_LEFTFIRST)
! 	if (ison(glob_flags, IS_GETTING))
! 	    print("Finish editing letter first");
! 	else
! 	    (void) func_key(ID);
      else if (isascii(ID) && (ison(glob_flags, IS_GETTING))) {
  	/*
  	 * If editing a letter, then enter the keys typed.
--- 62,81 ----
  	msg_rect = *((Rect *) window_get(msg_sw, WIN_RECT));
  	crt = msg_rect.r_height / l_height(curfont);
      }
!     else if (ID >= KEY_LEFTFIRST) {
! 	input_imnull(&im);
! 	win_keymap_set_imask_from_std_bind(&im, ACTION_OPEN);
! 	win_keymap_set_imask_from_std_bind(&im, ACTION_CLOSE);
! 	win_keymap_set_imask_from_std_bind(&im, ACTION_FRONT);
! 	win_keymap_set_imask_from_std_bind(&im, ACTION_BACK);
! 	win_keymap_set_imask_from_std_bind(&im, ACTION_PROPS);
! 	if (!win_getinputcodebit(&im, ID)) {
! 	    if (ison(glob_flags, IS_GETTING))
! 		print("Finish editing letter first");
! 	    else
! 		(void) func_key(ID);
! 	}
!     }
      else if (isascii(ID) && (ison(glob_flags, IS_GETTING))) {
  	/*
  	 * If editing a letter, then enter the keys typed.
***************
*** 110,115 ****
--- 120,126 ----
  {
      static int	which_cursor;
      int 	line;
+     Inputmask	im;
  
      if (ID == WIN_REPAINT) {
  	if (is_iconic != (int) window_get(tool, FRAME_CLOSED)) {
***************
*** 145,152 ****
  	return;
  
      line = event_y(event) / l_height(DEFAULT);
!     if (ID >= KEY_LEFTFIRST)
! 	(void) func_key(ID);
      else if (!msg_cnt || n_array[line] > msg_cnt)
  	if (!msg_cnt)
  	    print("-- You have no messages -- ");
--- 156,171 ----
  	return;
  
      line = event_y(event) / l_height(DEFAULT);
!     if (ID >= KEY_LEFTFIRST) {
! 	input_imnull(&im);
! 	win_keymap_set_imask_from_std_bind(&im, ACTION_OPEN);
! 	win_keymap_set_imask_from_std_bind(&im, ACTION_CLOSE);
! 	win_keymap_set_imask_from_std_bind(&im, ACTION_FRONT);
! 	win_keymap_set_imask_from_std_bind(&im, ACTION_BACK);
! 	win_keymap_set_imask_from_std_bind(&im, ACTION_PROPS);
! 	if (!win_getinputcodebit(&im, ID))
! 	    (void) func_key(ID);
!     }
      else if (!msg_cnt || n_array[line] > msg_cnt)
  	if (!msg_cnt)
  	    print("-- You have no messages -- ");



The following is a patch to what appears to be an real tool mode bug.  I
think this one actually bit me the other day.  I hit the done button
just about the time new mail was coming in and mushtool hung trying to
read from fd 0.  I don't really know if there should be some explicit
tool mode code here, but the original is clearly wrong.


*** ORIGmsgs.c	Thu Sep  7 11:26:30 1989
--- msgs.c	Wed Nov 22 00:57:28 1989
***************
*** 253,263 ****
  	    char buf[16];
  	    if (iscurses)
  		putchar('\n'), turnon(glob_flags, CNTD_CMD);
! 	    if (!istool)
  		print("%s [n] ", prompt);
  		buf[0] = 0;
  		if (!Getstr(buf, sizeof (buf), 0) || lower(*buf) != 'y')
  		    return 0;
  	}
      }
      first = 0;
--- 253,264 ----
  	    char buf[16];
  	    if (iscurses)
  		putchar('\n'), turnon(glob_flags, CNTD_CMD);
! 	    if (!istool) {
  		print("%s [n] ", prompt);
  		buf[0] = 0;
  		if (!Getstr(buf, sizeof (buf), 0) || lower(*buf) != 'y')
  		    return 0;
+ 	    }
  	}
      }
      first = 0;


I found the above when I was looking to see how hard it would be to modify
mushtool so that it would go iconic before actually saving the mail folder.
The present implementation is annoying if you have a large mail folder and
you want to get mush out of the way to make some room on your screen.
Unfortunately, this looks like one of those things that is not very easy
to implement.

Odds and Ends:

I noticed that if you hit the update button and your folder is already
up to date, mushtool doesn't ask if you want to update the folder, but
it does go off and think for quit a while.  It's not obvious what it is
doing.

I noticed in msgs.c that the permission checking on the folders is done
by checking the owner permission bits returned by stat().  This is not
correct if the folder is owned by another user.  Shouldn't the access()
system call be used instead?

I'm really impressed by the different mouse buttons getting highlighted
on the cursor in the header window ;-).
--
Don "Truck" Lewis                                     Harris Semiconductor
Internet (if you're lucky): del@semi.harris-atd.com   PO Box 883   MS 62A-028
Internet (if not): del%thrush@trantor.harris-atd.com  Melbourne, FL  32901
UUCP (works): rutgers!soleil!thrush!del               Phone: (407) 729-5205

argv%turnpike@Sun.COM (Dan Heller) (11/23/89)

In article <1989Nov22.064230.13339@semi.harris-atd.com> del@thrush.semi.harris-atd.com (Don Lewis) writes:
> But in the mean time, the function keys are the only way to do useful
> operations that are not possible given the buttons and menus.  I use
> one of the function keys to do "mail -f".  So see below for a throwaway
> patch for this until we get the real thing.

well, function keys haven't -really- gone away yet (depending on your
version).  the fkey command still exists in most versions so you can 
still specify function key bindings in your .mushrc.  And you can use
these function keys in the headers window.  In the meantime, your fix
is a fine temporary workaround.

Incidentally, you included the same patch twice, so anyone attempting
to apply the patch might find interesting errors :-)

> The following is a patch to what appears to be an real tool mode bug.  I
A fine fix for your current version, but this code has changed
somewhat in the latest version.

> I found the above when I was looking to see how hard it would be to modify
> mushtool so that it would go iconic before actually saving the mail folder.
You could do it by using a notify_interpose_proc(), but then you get into
hairy details.

> Odds and Ends:
> I noticed that if you hit the update button and your folder is already
> up to date, mushtool doesn't ask if you want to update the folder, but
> it does go off and think for quit a while.  It's not obvious what it is
> doing.
What -- don't you like to just go off and think for a while? :-)
As noted above, this portion of the code has changed a lot so this
problem may have gone away.  At least, I haven't noticed it.

> I noticed in msgs.c that the permission checking on the folders is done
> by checking the owner permission bits returned by stat().  This is not
> correct if the folder is owned by another user.  Shouldn't the access()
> system call be used instead?
Mush uses its own call: Access() --this is because the access() system
call has an undocumented feature which prohibits use when suid to someone
else.  However, in the case you're talking about, you may be right that
the stat() call should check the file's uid and check the appropriate
permission bits as well..

> I'm really impressed by the different mouse buttons getting highlighted
> on the cursor in the header window ;-).
The first one who ever appreciated it :-|  Everyone else asks:
"why the hell do all the mouse buttons blink when I move the cursor
in the headers window?"
dan

schaefer@ogccse.ogc.edu (Barton E. Schaefer) (11/23/89)

In article <1989Nov22.064230.13339@semi.harris-atd.com>
    del@semi.harris-atd.com (Don Lewis) writes:
} In article <128221@sun.Eng.Sun.COM> argv@sun.UUCP (Dan Heller) writes:
[ >I (Bart) wrote: ]
} >> The function keys are going away altogether as I understand it.
} >As a replacement (someday), there is going to be a "command"
} >window similar to dbxtool, maybe.  That is, you will be able to enter
} >mush commands directly from a special window or prompt.
} 
} But in the mean time, the function keys are the only way to do useful
} operations that are not possible given the buttons and menus.

WARNING!  AWFUL CRUFTY UNDOCUMENTED USE-AT-YOUR-OWN-RISK TRICK FOLLOWS!

You can actually execute *any* line-mode command from tool mode, but be
warned that if the command produces output you might be in deep doodie.
Also, the number of commands for which this trick works has shrunk with
every new toolmode patch since 6.5.6; I have no idea which ones break any
given version, but I believe "echo" breaks all of them ....

Here's the trick:

Select the <Folder> button, which changes the Filename: prompt to Folder:.

Backspace over any file name that may already be on the prompt line.

Type a semicolon followed by any line mode command, e.g:

    Folder: ;mail -f

Type a carriage return and hold your breath.  Either the command will work
exactly as it should, or mush will dump core; there is no in-between.

} The following is a patch to what appears to be an real tool mode bug.  I
} think this one actually bit me the other day.  I hit the done button
} just about the time new mail was coming in and mushtool hung trying to
} read from fd 0.  I don't really know if there should be some explicit
} tool mode code here, but the original is clearly wrong.

[ Diff deleted ]

This piece of code has changed a bit; the indentation was still wrong
(I just corrected it), but the tool mode code needed had been added.

} I noticed that if you hit the update button and your folder is already
} up to date, mushtool doesn't ask if you want to update the folder, but
} it does go off and think for quit a while.  It's not obvious what it is
} doing.

I believe it's re-reading the folder.  Update always causes a complete
re-load of the folder, even if nothing was written back.

} I noticed in msgs.c that the permission checking on the folders is done
} by checking the owner permission bits returned by stat().  This is not
} correct if the folder is owned by another user.  Shouldn't the access()
} system call be used instead?

Sigh.  It's a no-win situation.  If you use access(), the result is wrong
when you are running setuid (with different real and effective uids).  If
you use stat(), you don't know which set of permission bits to check
without doing a *lot* of extra work.  Fortunately, the primary use of the
function that calls stat() is to determine whether a file exists, and in
those cases where the bits actually are important, the code is redundant
and catches the permissions mismatch at a later stage.
-- 
Bart Schaefer          "Sometimes I think the surest sign that intelligent life
                        exists elsewhere in the universe is that none of it has
schaefer@cse.ogi.edu        tried to contact us."                     -- Calvin
(used to be cse.ogc.edu)