zsh-workers
 help / color / mirror / code / Atom feed
From: Oliver Kiddle <opk@zsh.org>
To: Bart Schaefer <schaefer@brasslantern.com>
Cc: Zsh workers <zsh-workers@zsh.org>
Subject: Re: PATCH: separate watch/log functionality out into a module
Date: Sat, 06 Nov 2021 01:17:47 +0100	[thread overview]
Message-ID: <43155-1636157867.215533@Ne4o.Iiwb.hwym> (raw)
In-Reply-To: <CAH+w=7bm60WQw0scd9boMKDrcQjYQh89d-qA-yqPxKOX6uQqMw@mail.gmail.com>

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.


  reply	other threads:[~2021-11-06  0:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-29 22:15 Oliver Kiddle
2021-11-01  0:06 ` Oliver Kiddle
2021-11-01  3:11   ` Bart Schaefer
2021-11-06  0:17     ` Oliver Kiddle [this message]
2021-11-06 20:53       ` Bart Schaefer
2021-11-28 20:56         ` Oliver Kiddle
2021-11-29  4:31           ` Bart Schaefer
2021-11-09 10:27       ` Jun. T
2021-11-09 21:09         ` Daniel Shahaf
2021-11-11 11:06 ` Jun T
2021-11-11 17:08   ` Oliver Kiddle
2021-11-12  9:19     ` Jun T

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=43155-1636157867.215533@Ne4o.Iiwb.hwym \
    --to=opk@zsh.org \
    --cc=schaefer@brasslantern.com \
    --cc=zsh-workers@zsh.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).