PATCH: preexec sometimes gets wrong command

PATCH: preexec sometimes gets wrong command

[Wayne Davison]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1131 bytes --]

I just noticed that preexec gets passed the wrong command when the
current command is discarded from the history (as is often the case
when the user is ignoring functions, history commands, and/or lines
that start with a space).  In this case, preexec got passed the
previous command from the history buffer.  I couldn't think of any
other way to grab the actual text that the user typed, so I decided to
set the first parameter to an empty string when this happens.  Perhaps
a better solution would be to pass the current (expanded) command
string?

Speaking of expanded commands, I was really hoping to have access to
the alias-expanded command string for my preexec processing, so I also
decided to pass this information to the preexec function as the second
parameter.  It would be very easy to tweak my patch to supply this
value as both parameters (instead of the empty string in the first)
when we don't have the history text for the first argument.

The following patch implements these two changes and tweaks the doc.

Let me know what you think.  If people approve, I'll be glad to check
the changes into CVS.

..wayne..

[-- Attachment #2: preexec changes --]
[-- Type: TEXT/PLAIN, Size: 1382 bytes --]

Index: Doc/Zsh/func.yo
@@ -172,8 +172,11 @@
 findex(preexec)
 item(tt(preexec))(
 Executed just after a command has been read and is about to be
-executed.  If the history mechanism is active, the string to be
-executed is passed as an argument.
+executed.  If the history mechanism is active (and the line was not
+discarded from the history buffer), the string that the user typed is
+passed as the first argument, otherwise it is an empty string.  The
+actual command that will be executed (including expanded aliases) is
+passed as the second argument (and is always present).
 )
 item(tt(TRAP)var(NAL))(
 cindex(signals, trapping)
Index: Src/init.c
@@ -135,15 +135,22 @@
 	    if (toplevel && (preprog = getshfunc("preexec")) != &dummy_eprog) {
 		LinkList args;
 		int osc = sfcontext;
+		char *cmdstr = getpermtext(prog, NULL);
 
 		args = znewlinklist();
 		zaddlinknode(args, "preexec");
-		if (hist_ring)
+		/* If curline got dumped from the history, we don't know
+		 * what the user typed. */
+		if (hist_ring && curline.histnum == curhist)
 		    zaddlinknode(args, hist_ring->text);
+		else
+		    zaddlinknode(args, "");
+		zaddlinknode(args, cmdstr);
 
 		sfcontext = SFC_HOOK;
 		doshfunc("preexec", preprog, args, 0, 1);
 		sfcontext = osc;
+		zsfree(cmdstr);
 		freelinklist(args, (FreeFunc) NULL);
 		errflag = 0;
 	    }

Re: PATCH: preexec sometimes gets wrong command

[Bart Schaefer]

On Apr 9,  1:03pm, Wayne Davison wrote:
} 
} I just noticed that preexec gets passed the wrong command when the
} current command is discarded from the history [...] so I decided to
} set the first parameter to an empty string when this happens.  Perhaps
} a better solution would be to pass the current (expanded) command
} string?

I think the empty string is the right thing; the docs say "If the history
mechanism is active, ..." and using a leading space etc. is explicitly a
way to deactivate history for this command.

I'd actually vote for:

	$1	the history line, or empty
	$2	getjobtext(), or empty (is it ever?)
	$3	getpermtext() (what your patch passes as $2)
 

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: PATCH: preexec sometimes gets wrong command

[Wayne Davison]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 367 bytes --]

On Tue, 10 Apr 2001, Bart Schaefer wrote:
> I'd actually vote for:
>
> 	$1	the history line, or empty
> 	$2	getjobtext(), or empty (is it ever?)
> 	$3	getpermtext() (what your patch passes as $2)

Seems like a good idea to me, so I added the extra argument.
(FYI: getjobtext() will never be empty.)

Does anyone have any concerns about my committing this?

..wayne..

[-- Attachment #2: Second version of preexec tweak --]
[-- Type: TEXT/PLAIN, Size: 1599 bytes --]

Index: Doc/Zsh/func.yo
@@ -172,8 +172,14 @@
 findex(preexec)
 item(tt(preexec))(
 Executed just after a command has been read and is about to be
-executed.  If the history mechanism is active, the string to be
-executed is passed as an argument.
+executed.  If the history mechanism is active (and the line was not
+discarded from the history buffer), the string that the user typed is
+passed as the first argument, otherwise it is an empty string.  The
+actual command that will be executed (including expanded aliases) is
+passed in two differnt forms: the second argument is a single-line,
+size-limited version of the command (with things like function bodies
+elided); the third argument contains the full text what what is being
+executed.
 )
 item(tt(TRAP)var(NAL))(
 cindex(signals, trapping)
Index: Src/init.c
@@ -135,15 +135,23 @@
 	    if (toplevel && (preprog = getshfunc("preexec")) != &dummy_eprog) {
 		LinkList args;
 		int osc = sfcontext;
+		char *cmdstr;
 
 		args = znewlinklist();
 		zaddlinknode(args, "preexec");
-		if (hist_ring)
+		/* If curline got dumped from the history, we don't know
+		 * what the user typed. */
+		if (hist_ring && curline.histnum == curhist)
 		    zaddlinknode(args, hist_ring->text);
+		else
+		    zaddlinknode(args, "");
+		zaddlinknode(args, getjobtext(prog, NULL));
+		zaddlinknode(args, cmdstr = getpermtext(prog, NULL));
 
 		sfcontext = SFC_HOOK;
 		doshfunc("preexec", preprog, args, 0, 1);
 		sfcontext = osc;
+		zsfree(cmdstr);
 		freelinklist(args, (FreeFunc) NULL);
 		errflag = 0;
 	    }