Re: PATCH: separate watch/log functionality out into a module

[Oliver Kiddle <opk@xxxxxxx>]

On 31 Oct, Bart Schaefer wrote:

(reordering Bart's reply here)

> I think you're misinterpreting what's happening.  If I use gdb:

Yes, you're right so I retract most of what I said. I'm used to
just prefixing commands to export values but the assignment is, of
course, taking place in the initial shell.

> Hm.  What would the right behavior be?  Do we need a flag on the
> autoloaded parameter for whether the value is [not?] allowed to come
> from the environment?  PM_DONTIMPORT seems potentially relevant.

I'm not sure whether PM_DONTIMPORT really helps at all.
It is probably necessary this way but even with env, we do get:
  env options=foo zsh -df
  zsh: Can't add module parameter `options': parameter already exists

But for a parameter like WATCH, it'd ideally load the module and use the
imported value. Is it an important feature that $WATCH can come from the
environment?

With sh emulation we don't get that error message which was really what
I was concerned about.

> (this is all without your patches)

I don't think my patches will have changed anything in this regard other
than for what happens with $watch and $WATCH.

On further testing zsh/watch does need to better guard against one of
them coming from the environment. They can only be tied if both are
there. The patch below covers this case.

Can someone with a Mac confirm something: have they disabled the log
builtin because it clashed with an external command. I did have a link
for their code modifications but the link appears to be broken. A log(1)
man page does seem to exist based on a web search. If watch is a module,
hopefully they include it in future and only disable the autoloading.

Oliver

diff --git a/Src/Modules/watch.c b/Src/Modules/watch.c
index 02f0562fc..5ce604c63 100644
--- a/Src/Modules/watch.c
+++ b/Src/Modules/watch.c
@@ -640,8 +640,8 @@ static struct builtin bintab[] = {
 };
 
 static struct paramdef partab[] = {
-    PARAMDEF("WATCH", PM_TIED|PM_SCALAR|PM_SPECIAL, &watch, &colonarr_gsu),
-    PARAMDEF("watch", PM_TIED|PM_ARRAY|PM_SPECIAL, &watch, &vararray_gsu),
+    PARAMDEF("WATCH", PM_SCALAR|PM_SPECIAL, &watch, &colonarr_gsu),
+    PARAMDEF("watch", PM_ARRAY|PM_SPECIAL, &watch, &vararray_gsu),
 };
 
 static struct features module_features = {
@@ -679,12 +679,16 @@ int
 boot_(UNUSED(Module m))
 {
     static char const * const default_watchfmt = DEFAULT_WATCHFMT;
-    Param pm;
 
-    if ((pm = (Param) paramtab->getnode(paramtab, "watch")))
-	pm->ename = "WATCH";
-    if ((pm = (Param) paramtab->getnode(paramtab, "WATCH")))
-	pm->ename = "watch";
+    Param pma = (Param) paramtab->getnode(paramtab, "watch");
+    Param pms = (Param) paramtab->getnode(paramtab, "WATCH");
+    if (pma && pms && pma->u.arr == watch && pms->u.arr == watch) {
+	/* only tie the two parameters if both were added */
+	pma->ename = "WATCH";
+	pms->ename = "watch";
+	pma->node.flags |= PM_TIED;
+	pms->node.flags |= PM_TIED;
+    }
     watch = mkarray(NULL);
 
     /* These two parameters are only set to defaults if not set.