From f51e54828e4d4aff91c3d3a2dd0e353022e7b164 Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Fri, 10 Jun 2022 11:16:55 +0100
Subject: [PATCH] sh_exec(): do not turn off editors for path-bound builtins

Here's another one from the "question everything" department.

Every time ksh executes a built-in, it checks whether it is bound
to a path (the name contains a '/') and if so, it turns off the vi,
emacs and gmacs shell options before executing it. After executing
the built-ins, those shell options are restored again. The comment
is: "turn off editors for built-in versions of commands on PATH".

But why? Commands do not and cannot ever use the command line
editors. They are only ever executed on an interactive shell
*between* running commands. So turning them off during the
execution of a command makes no difference.

The only possible effect is that these commands cannot check if a
user has the vi, emacs or gmacs option on, or change these options
itself. But what purpose could that limitation possibly serve? And
what sense does it make to do this for path-bound built-ins only
and not for other commands? And what sense does it make to have
this limitation only for the editor shell options and not others?

This waste of CPU cycles has been there since the beginning of the
ksh93-history repo (1995) so if there was ever any good reason for
it, it has been lost in the mist of history. I'm 99.999% sure that
nothing will happen after deleting this.
---
 src/cmd/ksh93/sh/xec.c | 34 ----------------------------------
 1 file changed, 34 deletions(-)

diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c
index b998617ed..0b981d7ea 100644
--- a/src/cmd/ksh93/sh/xec.c
+++ b/src/cmd/ksh93/sh/xec.c
@@ -1231,34 +1231,11 @@ int sh_exec(register const Shnode_t *t, int flags)
 					int save_prompt;
 					int was_nofork = execflg?sh_isstate(SH_NOFORK):0;
 					struct checkpt *buffp = (struct checkpt*)stkalloc(sh.stk,sizeof(struct checkpt));
-#if SHOPT_VSH
-					volatile unsigned long was_vi=0;
-#endif
-#if SHOPT_ESH
-					volatile unsigned long was_emacs=0, was_gmacs=0;
-#endif
 					struct stat statb;
 					bp = &sh.bltindata;
 					save_ptr = bp->ptr;
 					save_data = bp->data;
 					memset(&statb, 0, sizeof(struct stat));
-					if(strchr(nv_name(np),'/'))
-					{
-						/*
-						 * disable editors for built-in
-						 * versions of commands on PATH
-						 */
-#if SHOPT_VSH
-						was_vi = sh_isoption(SH_VI);
-						sh_offoption(SH_VI);
-#endif
-#if SHOPT_ESH
-						was_emacs = sh_isoption(SH_EMACS);
-						was_gmacs = sh_isoption(SH_GMACS);
-						sh_offoption(SH_EMACS);
-						sh_offoption(SH_GMACS);
-#endif
-					}
 					if(execflg)
 						sh_onstate(SH_NOFORK);
 					sh_pushcontext(buffp,SH_JMPCMD);
@@ -1417,17 +1394,6 @@ int sh_exec(register const Shnode_t *t, int flags)
 					sh.bltinfun = 0;
 					if(buffp->olist)
 						free_list(buffp->olist);
-#if SHOPT_VSH
-					if(was_vi)
-						sh_onoption(SH_VI);
-					else
-#endif
-#if SHOPT_ESH
-					     if(was_emacs)
-						sh_onoption(SH_EMACS);
-					else if(was_gmacs)
-						sh_onoption(SH_GMACS);
-#endif
 					if(scope)
 						sh_unscope();
 					bp->ptr = (void*)save_ptr;