* r problem @ 2001-09-12 11:07 Tanaka Akira 2001-09-12 16:03 ` Bart Schaefer 0 siblings, 1 reply; 6+ messages in thread From: Tanaka Akira @ 2001-09-12 11:07 UTC (permalink / raw) To: zsh-workers I found a problem with a builtin command `r'. % zsh -f coulee% r r r r r ... r r r r zsh: job table full or recursion limit exceeded coulee% echo $ZSH_VERSION 4.0.2 coulee% -- Tanaka Akira ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: r problem 2001-09-12 11:07 r problem Tanaka Akira @ 2001-09-12 16:03 ` Bart Schaefer 2001-09-13 18:37 ` Wayne Davison 0 siblings, 1 reply; 6+ messages in thread From: Bart Schaefer @ 2001-09-12 16:03 UTC (permalink / raw) To: zsh-workers This looks like a bug in gethistent(). It's returning the current command as the previous command when the history should appear to be empty. Wayne? Another bug (?) is that the "r" command itself gets entered in the history even when hist_no_store is set. On the other hand, "history" and "fc" get left out of the history with hist_no_store even when they've been redifined as functions that have nothing to do with the history. Hrm. -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: r problem 2001-09-12 16:03 ` Bart Schaefer @ 2001-09-13 18:37 ` Wayne Davison 2001-09-14 0:52 ` Wayne Davison 0 siblings, 1 reply; 6+ messages in thread From: Wayne Davison @ 2001-09-13 18:37 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh-workers On Wed, 12 Sep 2001, Bart Schaefer wrote: > This looks like a bug in gethistent(). It was actually a problem with addhistnum() in how it was choosing to return boundary values. I changed this, and then checked everything that was calling addhistnum() to ensure that they should be able to handle the new behavior. > Another bug (?) is that the "r" command itself gets entered in the history > even when hist_no_store is set. Yes, I hadn't remembered to add that to the hist_no_store check. > On the other hand, "history" and "fc" get left out of the history with > hist_no_store even when they've been redefined as functions that have > nothing to do with the history. They are no longer omitted if you alias them, so that is a better way to go at the moment. With the advent of my HIST_TMPSTORE flag, I could change the builtins to set this value and get rid of the command-line scan. I'll look into this later to see if I think this is a good idea. ..wayne.. ---8<------8<------8<------8<---cut here--->8------>8------>8------>8--- Index: Src/builtin.c --- Src/builtin.c 2001/08/13 17:43:04 1.51 +++ Src/builtin.c 2001/09/13 18:12:12 @@ -1299,8 +1299,8 @@ if (last == -1) last = ops['l']? addhistnum(curline.histnum,-1,0) : first; if (first < firsthist()) - first = firsthist(); - if (last == -1) + first = firsthist() - (first == last); + if (last > curhist) last = (minflag) ? curhist : first; else if (last < first) last = first; @@ -1368,7 +1368,7 @@ if ((cmd = atoi(s))) { if (cmd < 0) cmd = addhistnum(curline.histnum,cmd,HIST_FOREIGN); - if (cmd >= curline.histnum) { + if (cmd < firsthist()) { zwarnnam("fc", "bad history number: %d", 0, cmd); return -1; } Index: Src/hist.c --- Src/hist.c 2001/08/07 19:53:19 1.31 +++ Src/hist.c 2001/09/13 18:12:13 @@ -845,7 +845,7 @@ if (n) he = movehistent(he, n, xflags); if (!he) - return dir < 0? firsthist() : curhist; + return dir < 0? firsthist() - 1 : curhist + 1; return he->histnum; } @@ -993,6 +993,8 @@ b += 8; if (*b == 'h' && strncmp(b, "history", 7) == 0 && (!b[7] || b[7] == ' ')) + return 1; + if (*b == 'r' && (!b[1] || b[1] == ' ')) return 1; if (*b == 'f' && b[1] == 'c' && b[2] == ' ' && b[3] == '-') { b += 3; ---8<------8<------8<------8<---cut here--->8------>8------>8------>8--- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: r problem 2001-09-13 18:37 ` Wayne Davison @ 2001-09-14 0:52 ` Wayne Davison 2001-09-15 4:49 ` Bart Schaefer 0 siblings, 1 reply; 6+ messages in thread From: Wayne Davison @ 2001-09-14 0:52 UTC (permalink / raw) To: Zsh Workers On Thu, 13 Sep 2001, Wayne Davison wrote: > With the advent of my HIST_TMPSTORE flag, I could change the builtins > to set this value and get rid of the command-line scan. I'll look > into this later to see if I think this is a good idea. It's not. That would be too much like what we used to have, and the side-effects of dumping a history entry from inside a builtin function are not something that I want to revisit. So, we currently have the problem that I can define a function named "r" and with HIST_NO_STORE set, invoking that function from the command-line drops the command from the history. One potential solution to this would be to change the history-line-drop code to lookup "r" in a function list and avoid dropping "r" (but not "builtin r") if we find it. I don't know the function code well enough to know if there is an inexpensive call that should_ignore_line() could call for this or not. Another solution is to leave things alone and let people use an alias to redefine "r". Like this: function r_func { echo foo } alias r=r_func That's not as nice, but it works. Comments? ..wayne.. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: r problem 2001-09-14 0:52 ` Wayne Davison @ 2001-09-15 4:49 ` Bart Schaefer 2001-09-15 6:26 ` Wayne Davison 0 siblings, 1 reply; 6+ messages in thread From: Bart Schaefer @ 2001-09-15 4:49 UTC (permalink / raw) To: Zsh Workers On Sep 13, 5:52pm, Wayne Davison wrote: } } One potential solution to this would be to change the history-line-drop } code to lookup "r" in a function list and avoid dropping "r" (but not } "builtin r") if we find it. I don't know the function code well enough } to know if there is an inexpensive call that should_ignore_line() could } call for this or not. It's just a hash lookup: if (shfunctab->getnode(shfunctab, "r")) { /* It's a function */ } As it would only be happening after the command name had already been compared to (fc|history|r), I don't think it's prohibitively expensive, though it's exactly what exec.c is going to be doing a few instructions later. -- 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: r problem 2001-09-15 4:49 ` Bart Schaefer @ 2001-09-15 6:26 ` Wayne Davison 0 siblings, 0 replies; 6+ messages in thread From: Wayne Davison @ 2001-09-15 6:26 UTC (permalink / raw) To: Bart Schaefer; +Cc: Zsh Workers On Sat, 15 Sep 2001, Bart Schaefer wrote: > It's just a hash lookup: Excellent. That saves me some poking around that I hadn't gotten around to doing. Here's a patch that seems to fix things up quite well. ..wayne.. ---8<------8<------8<------8<---cut here--->8------>8------>8------>8--- Index: Src/hist.c --- Src/hist.c 2001/09/13 18:19:11 1.32 +++ Src/hist.c 2001/09/15 06:22:08 @@ -989,14 +989,20 @@ if (isset(HISTNOSTORE)) { char *b = getjobtext(prog, NULL); - if (*b == 'b' && strncmp(b, "builtin ", 8) == 0) + int saw_builtin; + if (*b == 'b' && strncmp(b,"builtin ",8) == 0) { b += 8; - if (*b == 'h' && strncmp(b, "history", 7) == 0 - && (!b[7] || b[7] == ' ')) + saw_builtin = 1; + } else + saw_builtin = 0; + if (*b == 'h' && strncmp(b,"history",7) == 0 && (!b[7] || b[7] == ' ') + && (saw_builtin || !shfunctab->getnode(shfunctab,"history"))) return 1; - if (*b == 'r' && (!b[1] || b[1] == ' ')) + if (*b == 'r' && (!b[1] || b[1] == ' ') + && (saw_builtin || !shfunctab->getnode(shfunctab,"r"))) return 1; - if (*b == 'f' && b[1] == 'c' && b[2] == ' ' && b[3] == '-') { + if (*b == 'f' && b[1] == 'c' && b[2] == ' ' && b[3] == '-' + && (saw_builtin || !shfunctab->getnode(shfunctab,"fc"))) { b += 3; do { if (*++b == 'l') ---8<------8<------8<------8<---cut here--->8------>8------>8------>8--- ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2001-09-15 6:26 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2001-09-12 11:07 r problem Tanaka Akira 2001-09-12 16:03 ` Bart Schaefer 2001-09-13 18:37 ` Wayne Davison 2001-09-14 0:52 ` Wayne Davison 2001-09-15 4:49 ` Bart Schaefer 2001-09-15 6:26 ` Wayne Davison
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).