* Proposal (with code) to fix breaking of history widgets on reset-prompt @ 2019-03-23 13:09 ` Roman Perepelitsa 2019-03-28 11:20 ` Peter Stephenson 0 siblings, 1 reply; 5+ messages in thread From: Roman Perepelitsa @ 2019-03-23 13:09 UTC (permalink / raw) To: zsh-workers [-- Attachment #1: Type: text/plain, Size: 2900 bytes --] Hi, If your [up] key is bound to up-line-or-beginning-search, like most people have it, try the following experiment. Type sleep 6& and hit [enter]. Then quickly press [up] and wait for the job to finish. Once it finishes, press [up] again. Oh no! History doesn’t work. You are supposed to see the command you’d typed before sleep 6& but you are likely still seeing sleep 6& . Here’s what’s going on. When jobs finish, zle .reset-prompt is called to re-expand and display prompt. This sets LASTWIDGET to .reset-prompt. When you press [up] after that, up-line-or-beginning-search will behave as if you’ve typed sleep 6& in your prompt and then pressed [up]. That is, it’ll search for the previous command in your history that starts with sleep 6&. up-line-or-beginning-search breaks whenever reset-prompt is called. This happens when jobs finish and also when ZSH themes with async support want to refresh prompt after collecting some data in the background. For example, Powerlevel10k <https://github.com/romkatv/powerlevel10k> theme shows a greyed-out prompt when entering a large git repo and then updates it once it figures out git status in the background. If the user has pressed [up] while the prompt was grey, they’ll be surprised to find that [up] doesn’t work as expected once prompt updates. One potential fix for this issue is to change reset-prompt widget so that it keeps LASTWIDGET unchanged. I’ve implemented this change in https://github.com/romkatv/zsh/tree/gentle-reset-prompt. Diff against upstream: https://github.com/zsh-users/zsh/compare/master...romkatv:gentle-reset-prompt . The same branch also adds -W option to zle widget command. This option instructs zle to keep LASTWIDGET unchanged. I added it because it’s a natural extension of what I’ve done with .reset-prompt and because it has solved a long-standing issue I had in my zsh config. The long-stating problem is that I want my [up] key to behave as up-line-or-beginning-search with local history turned on, so that it doesn’t see history from other sessions, and my [ctrl+up] key as up-line-or-beginning-search with local history turned off. This is very difficult to achieve. The only solution I’ve found is to fork up-line-or-beginning-search and edit its source code. You can see it in my dotfiles <https://github.com/romkatv/dotfiles-public/blob/88218b6fa6801ad4448b04a6893966b9221c1491/bin/local-history.zsh>. With the new -W flag that I added to zle widget this problem has a clear solution. Like this: # This zle widget is the same as the standard up-line-or-beginning-search# but restricted to local history. function up-line-or-beginning-search-local() { zle .set-local-history -W 1 up-line-or-beginning-search zle .set-local-history -W 0 } zle -N up-line-or-beginning-search-local What do you think? Roman. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Proposal (with code) to fix breaking of history widgets on reset-prompt 2019-03-23 13:09 ` Proposal (with code) to fix breaking of history widgets on reset-prompt Roman Perepelitsa @ 2019-03-28 11:20 ` Peter Stephenson 2019-04-09 18:45 ` Roman Perepelitsa 0 siblings, 1 reply; 5+ messages in thread From: Peter Stephenson @ 2019-03-28 11:20 UTC (permalink / raw) To: zsh-workers > By the way, any chance you could take a look at > https://www.zsh.org/mla/workers//2019/msg00204.html or ping someone who > might? On Sat, 2019-03-23 at 14:09 +0100, Roman Perepelitsa wrote: > If your [up] key is bound to up-line-or-beginning-search, like most people > have it, try the following experiment. Type sleep 6& and hit [enter]. Then > quickly press [up] and wait for the job to finish. Once it finishes, press > [up] again. Oh no! History doesn’t work. You are supposed to see the > command you’d typed before sleep 6& but you are likely still seeing sleep 6& > . > > Here’s what’s going on. When jobs finish, zle .reset-prompt is called to > re-expand and display prompt. This sets LASTWIDGET to .reset-prompt. When > you press [up] after that, up-line-or-beginning-search will behave as if > you’ve typed sleep 6& in your prompt and then pressed [up]. That is, it’ll > search for the previous command in your history that starts with sleep > 6&. >... > One potential fix for this issue is to change reset-prompt widget so that > it keeps LASTWIDGET unchanged. I’ve implemented this change in > https://github.com/romkatv/zsh/tree/gentle-reset-prompt. Diff against > upstream: > https://github.com/zsh-users/zsh/compare/master...romkatv:gentle-reset-prompt > . Don't see any problem with doing that. I suppose we really need a list of widgets that should have this behaviour. Ideally, this would be encapsulated as anything that happens not as part of a user command --- in your example, that's the code executed at the end of the job (I think as a result of setting "resetneeded" in trahszle()) that calls last-prompt, rather than last-prompt itself. But that could be more major surgery, with largely the same effect since it's that widget that's causing the problem. So I'm perfectly happy to get this fixed by the present means. If you can produce this as a single change (with appropriate use of rebase -i or whatever) we can apply it. A patch to the list would be fine, since other people get to see it, but I don't mind cherry-picking / rebasing from your repo if it's a single squashed change somewhere. > The same branch also adds -W option to zle widget command. This option > instructs zle to keep LASTWIDGET unchanged. I added it because it’s a > natural extension of what I’ve done with .reset-prompt and because it has > solved a long-standing issue I had in my zsh config. The functionality certainly seems useful. It might be easier simply to make LASTWIDGET read-write. Then you could do local LASTWIDGET=$LASTWIDGET (you only need the assignment if you expect something down below to look at it) and no new syntax is needed. The save/restore just uses existing mechanisms intended for doing that with variables, which seems more efficient, too. pws ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Proposal (with code) to fix breaking of history widgets on reset-prompt 2019-03-28 11:20 ` Peter Stephenson @ 2019-04-09 18:45 ` Roman Perepelitsa 2019-04-10 8:46 ` Peter Stephenson 0 siblings, 1 reply; 5+ messages in thread From: Roman Perepelitsa @ 2019-04-09 18:45 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers [-- Attachment #1.1: Type: text/plain, Size: 768 bytes --] On Thu, Mar 28, 2019 at 12:21 PM Peter Stephenson <p.stephenson@samsung.com> wrote: > Don't see any problem with doing that. I suppose we really need a > list of widgets that should have this behaviour. > > [...] > > If you can produce this as a single change (with appropriate use of > rebase -i or whatever) we can apply it. A patch to the list would be > fine, since other people get to see it, but I don't mind cherry-picking > / rebasing from your repo if it's a single squashed change somewhere. > Cool! I can do that. Here's a branch with a single commit that modifies reset-prompt so that it doesn't alter LASTWIDGET: https://github.com/zsh-users/zsh/compare/master...romkatv:gentle-reset-prompt2 . Please find a patch from it in the attachment. Roman. [-- Attachment #1.2: Type: text/html, Size: 1278 bytes --] [-- Attachment #2: gentle-reset-prompt.patch --] [-- Type: text/x-patch, Size: 5495 bytes --] diff --git a/Doc/Zsh/zle.yo b/Doc/Zsh/zle.yo index c2b9f5430..0986e5390 100644 --- a/Doc/Zsh/zle.yo +++ b/Doc/Zsh/zle.yo @@ -2415,9 +2415,12 @@ directory, or changes to the value of variables referred to by the prompt). Otherwise, the prompt is only expanded each time zle starts, and -when the display as been interrupted by output from another part of the +when the display has been interrupted by output from another part of the shell (such as a job notification) which causes the command line to be reprinted. + +tt(reset-prompt) doesn't alter the special parameter tt(LASTWIDGET). + ) tindex(send-break) item(tt(send-break) (tt(^G ESC-^G)) (unbound) (unbound))( diff --git a/Src/Zle/iwidgets.list b/Src/Zle/iwidgets.list index 58310cd74..c95c7a491 100644 --- a/Src/Zle/iwidgets.list +++ b/Src/Zle/iwidgets.list @@ -99,7 +99,7 @@ "recursive-edit", recursiveedit, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL "redisplay", redisplay, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL "redo", redo, ZLE_KEEPSUFFIX -"reset-prompt", resetprompt, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL +"reset-prompt", resetprompt, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL | ZLE_NOTCOMMAND | ZLE_NOLAST "reverse-menu-complete", reversemenucomplete, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_ISCOMP "run-help", processcmd, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL "select-a-word", selectword, ZLE_KEEPSUFFIX diff --git a/Src/Zle/zle.h b/Src/Zle/zle.h index f06c56483..609493f8c 100644 --- a/Src/Zle/zle.h +++ b/Src/Zle/zle.h @@ -217,6 +217,7 @@ struct widget { #define ZLE_ISCOMP (1<<11) /* usable for new style completion */ #define WIDGET_INUSE (1<<12) /* widget is in use */ #define WIDGET_FREE (1<<13) /* request to free when no longer in use */ +#define ZLE_NOLAST (1<<14) /* widget should not alter lbindk */ /* thingies */ diff --git a/Src/Zle/zle_main.c b/Src/Zle/zle_main.c index 71930f76b..d54e928a6 100644 --- a/Src/Zle/zle_main.c +++ b/Src/Zle/zle_main.c @@ -1073,7 +1073,7 @@ redrawhook(void) * temporarily reset state for special variable handling etc. */ incompfunc = 0; - execzlefunc(initthingy, args, 1); + execzlefunc(initthingy, args, 1, 0); incompfunc = old_incompfunc; /* Restore errflag and retflag as zlecallhook() does */ @@ -1136,7 +1136,7 @@ zlecore(void) eofsent = 1; break; } - if (execzlefunc(bindk, zlenoargs, 0)) { + if (execzlefunc(bindk, zlenoargs, 0, 0)) { handlefeep(zlenoargs); if (eofsent) break; @@ -1386,7 +1386,7 @@ execimmortal(Thingy func, char **args) { Thingy immortal = rthingy_nocreate(dyncat(".", func->nam)); if (immortal) - return execzlefunc(immortal, args, 0); + return execzlefunc(immortal, args, 0, 0); return 1; } @@ -1398,13 +1398,14 @@ execimmortal(Thingy func, char **args) /**/ int -execzlefunc(Thingy func, char **args, int set_bindk) +execzlefunc(Thingy func, char **args, int set_bindk, int set_lbindk) { int r = 0, ret = 0, remetafy = 0; int nestedvichg = vichgflag; int isrepeat = (viinrepeat == 3); Widget w; Thingy save_bindk = bindk; + Thingy save_lbindk = lbindk; if (set_bindk) bindk = func; @@ -1412,6 +1413,8 @@ execzlefunc(Thingy func, char **args, int set_bindk) unmetafy_line(); remetafy = 1; } + if (set_lbindk) + refthingy(save_lbindk); if (isrepeat) viinrepeat = 2; @@ -1535,7 +1538,10 @@ execzlefunc(Thingy func, char **args, int set_bindk) redup(osi, 0); } } - if (r) { + if (set_lbindk) { + unrefthingy(lbindk); + lbindk = save_lbindk; + } else if (r) { unrefthingy(lbindk); refthingy(func); lbindk = func; diff --git a/Src/Zle/zle_thingy.c b/Src/Zle/zle_thingy.c index 6b892b822..ce61db27b 100644 --- a/Src/Zle/zle_thingy.c +++ b/Src/Zle/zle_thingy.c @@ -703,7 +703,7 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) { Thingy t; struct modifier modsave = zmod; - int ret, saveflag = 0, setbindk = 0, remetafy; + int ret, saveflag = 0, setbindk = 0, setlbindk, remetafy; char *wname = *args++, *keymap_restore = NULL, *keymap_tmp; if (!wname) @@ -787,7 +787,8 @@ bin_zle_call(char *name, char **args, UNUSED(Options ops), UNUSED(char func)) * a vi range to detect a repeated key */ setbindk = setbindk || (t->widget && (t->widget->flags & (WIDGET_INT | ZLE_VIOPER)) == WIDGET_INT); - ret = execzlefunc(t, args, setbindk); + setlbindk = t->widget && (t->widget->flags & ZLE_NOLAST) == ZLE_NOLAST; + ret = execzlefunc(t, args, setbindk, setlbindk); unrefthingy(t); if (saveflag) zmod = modsave; diff --git a/Src/Zle/zle_utils.c b/Src/Zle/zle_utils.c index c6df3d89c..0277d4917 100644 --- a/Src/Zle/zle_utils.c +++ b/Src/Zle/zle_utils.c @@ -1733,7 +1733,7 @@ zlecallhook(char *name, char *arg) args[0] = arg; args[1] = NULL; - execzlefunc(thingy, args, 1); + execzlefunc(thingy, args, 1, 0); unrefthingy(thingy); /* Retain any user interrupt error status */ diff --git a/Src/Zle/zle_vi.c b/Src/Zle/zle_vi.c index a5ff9200c..0f198d0e8 100644 --- a/Src/Zle/zle_vi.c +++ b/Src/Zle/zle_vi.c @@ -216,7 +216,7 @@ getvirange(int wf) * a number of lines is used. If the function used * returns 1, we fail. */ - if ((k2 == bindk) ? dovilinerange() : execzlefunc(k2, zlenoargs, 1)) + if ((k2 == bindk) ? dovilinerange() : execzlefunc(k2, zlenoargs, 1, 0)) ret = -1; if (viinrepeat) zmult = mult1; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Proposal (with code) to fix breaking of history widgets on reset-prompt 2019-04-09 18:45 ` Roman Perepelitsa @ 2019-04-10 8:46 ` Peter Stephenson 2019-04-10 8:51 ` Roman Perepelitsa 0 siblings, 1 reply; 5+ messages in thread From: Peter Stephenson @ 2019-04-10 8:46 UTC (permalink / raw) To: zsh-workers On Tue, 2019-04-09 at 20:45 +0200, Roman Perepelitsa wrote: > On Thu, Mar 28, 2019 at 12:21 PM Peter Stephenson <p.stephenson@samsung.com> wrote: > > Don't see any problem with doing that. I suppose we really need a > > list of widgets that should have this behaviour. > > > > [...] > > > > If you can produce this as a single change (with appropriate use of > > rebase -i or whatever) we can apply it. A patch to the list would be > > fine, since other people get to see it, but I don't mind cherry-picking > > / rebasing from your repo if it's a single squashed change somewhere. > Cool! I can do that. > > Here's a branch with a single commit that modifies reset-prompt so that it > doesn't alter > LASTWIDGET: https://github.com/zsh-users/zsh/compare/master...romkatv:gentle-reset-prompt2. > > Please find a patch from it in the attachment. Thanks, I've applied this. pws ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Proposal (with code) to fix breaking of history widgets on reset-prompt 2019-04-10 8:46 ` Peter Stephenson @ 2019-04-10 8:51 ` Roman Perepelitsa 0 siblings, 0 replies; 5+ messages in thread From: Roman Perepelitsa @ 2019-04-10 8:51 UTC (permalink / raw) To: Peter Stephenson; +Cc: zsh-workers [-- Attachment #1: Type: text/plain, Size: 464 bytes --] On Wed, Apr 10, 2019 at 10:46 AM Peter Stephenson <p.stephenson@samsung.com> wrote: > > Thanks, I've applied this. > Thanks, Peter! There is a bug in completion menu I've reported in https://www.zsh.org/mla/workers/2019/msg00161.html but haven't got a reply. Unlike zle code, completion code in zsh looks very complex to me, so I couldn't figure out how to attempt to fix this. Could you give me any pointers or poke people who would know where to dig? Roman. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-10 8:52 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20190323131052epcas1p4ee9c963395adaa3c2bb560e1a290f6bd@epcas1p4.samsung.com> 2019-03-23 13:09 ` Proposal (with code) to fix breaking of history widgets on reset-prompt Roman Perepelitsa 2019-03-28 11:20 ` Peter Stephenson 2019-04-09 18:45 ` Roman Perepelitsa 2019-04-10 8:46 ` Peter Stephenson 2019-04-10 8:51 ` Roman Perepelitsa
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).