inherit OLDPWD from environment variable

inherit OLDPWD from environment variable

[Franklin Yu]

Hi folks, this could be either a bug report or feature request.

When I start Zsh with $OLDPWD set to a path different from $PWD, Zsh overwrites
$OLDPWD with $PWD. Is this expected, or is it a side effect of making sure
$OLDPWD is non-empty?

Re: inherit OLDPWD from environment variable

[Peter Stephenson]

> On 17/07/2024 08:59 BST Franklin Yu <franklinyu@hotmail.com> wrote:
> Hi folks, this could be either a bug report or feature request.
> 
> When I start Zsh with $OLDPWD set to a path different from $PWD, Zsh overwrites
> $OLDPWD with $PWD. Is this expected, or is it a side effect of making sure
> $OLDPWD is non-empty?

I think it's just making sure OLDPWD is set to something on
initialisation.  The only comment says "initialize `OLDPWD' = `PWD'".

I can't think of any obvious objection to importing a value from the
environment and only setting the variable to $PWD if that was empty.
Until the directory within the shell changes its value is pretty
arbitrary.

pws

Re: inherit OLDPWD from environment variable

[Peter Stephenson]

> On 21/07/2024 20:30 BST Peter Stephenson > Looks fine to me, I'll commit it assuming nobody comes up with any gotchas.

One correction.

diff --git a/Src/init.c b/Src/init.c
index ec21521b1..0aecb5db9 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1245,7 +1245,11 @@ setupvals(char *cmd, char *runscript, char *zsh_name)
 	pwd = metafy(zgetcwd(), -1, META_DUP);
     }
 
-    oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */
+    oldpwd = zgetenv("OLDPWD");
+    if (oldpwd == NULL)
+        oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */
+    else
+	oldpwd = ztrdup(oldpwd);
 
     inittyptab();     /* initialize the ztypes table */
     initlextabs();    /* initialize lexing tables    */

Re: inherit OLDPWD from environment variable

[Peter Stephenson]

> On 20/07/2024 08:56 BST Franklin Yu <franklinyu@hotmail.com> wrote:
> Good catch. Indeed Bash accepts empty value. Then it becomes
> 
> diff --git a/Src/init.c b/Src/init.c
> index ec21521b1..092273228 100644
> --- a/Src/init.c
> +++ b/Src/init.c
> @@ -1245,7 +1245,9 @@ setupvals(char *cmd, char *runscript, char *zsh_name)
>         pwd = metafy(zgetcwd(), -1, META_DUP);
>      }
> 
> -    oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */
> +    oldpwd = zgetenv("OLDPWD");
> +    if (oldpwd == NULL)
> +        oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */
> 
>      inittyptab();     /* initialize the ztypes table */
>      initlextabs();    /* initialize lexing tables    */
> 

Looks fine to me, I'll commit it assuming nobody comes up with any gotchas.

pws

Re: inherit OLDPWD from environment variable

[Franklin Yu]

Good catch. Indeed Bash accepts empty value. Then it becomes

diff --git a/Src/init.c b/Src/init.c
index ec21521b1..092273228 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1245,7 +1245,9 @@ setupvals(char *cmd, char *runscript, char *zsh_name)
        pwd = metafy(zgetcwd(), -1, META_DUP);
     }

-    oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */
+    oldpwd = zgetenv("OLDPWD");
+    if (oldpwd == NULL)
+        oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */

     inittyptab();     /* initialize the ztypes table */
     initlextabs();    /* initialize lexing tables    */

________________________________________
From: Lawrence Velázquez <larryv@zsh.org>
Sent: 19 July 2024 19:46
To: Franklin Yu <franklinyu@hotmail.com>
Cc: zsh-workers@zsh.org <zsh-workers@zsh.org>
Subject: Re: inherit OLDPWD from environment variable
 
On Fri, Jul 19, 2024, at 9:55 PM, Franklin Yu wrote:
> -    oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */
> +    oldpwd = zgetenv("OLDPWD");
> +    if (oldpwd == NULL || oldpwd[0] == '\0')
> +        oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */

If we're going to begin importing OLDPWD from the environment, we
might as well align with other shells for consistency, which means
accepting a zero-length value as is.  This is how bash 5.2.26, dash
0.5.12, ksh93u+ 2012-08-01, mksh R59, and yash 2.56.1 all behave.

--
vq

Re: inherit OLDPWD from environment variable

[Lawrence Velázquez]

On Fri, Jul 19, 2024, at 9:55 PM, Franklin Yu wrote:
> -    oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */
> +    oldpwd = zgetenv("OLDPWD");
> +    if (oldpwd == NULL || oldpwd[0] == '\0')
> +        oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */

If we're going to begin importing OLDPWD from the environment, we
might as well align with other shells for consistency, which means
accepting a zero-length value as is.  This is how bash 5.2.26, dash
0.5.12, ksh93u+ 2012-08-01, mksh R59, and yash 2.56.1 all behave.

-- 
vq

Re: inherit OLDPWD from environment variable

[Franklin Yu]

> > On 17/07/2024 08:59 BST Franklin Yu <franklinyu@hotmail.com> wrote:
> > Hi folks, this could be either a bug report or feature request.
> >
> > When I start Zsh with $OLDPWD set to a path different from $PWD, Zsh overwrites
> > $OLDPWD with $PWD. Is this expected, or is it a side effect of making sure
> > $OLDPWD is non-empty?
> 
> I think it's just making sure OLDPWD is set to something on
> initialisation.  The only comment says "initialize `OLDPWD' = `PWD'".
> 
> I can't think of any obvious objection to importing a value from the
> environment and only setting the variable to $PWD if that was empty.
> Until the directory within the shell changes its value is pretty
> arbitrary.
> 
> pws

Good to hear that this idea works for you. Shall we adopt this idea, then? I
think it is as simple as:

diff --git a/Src/init.c b/Src/init.c
index ec21521b1..092273228 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1245,7 +1245,9 @@ setupvals(char *cmd, char *runscript, char *zsh_name)
        pwd = metafy(zgetcwd(), -1, META_DUP);
     }

-    oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */
+    oldpwd = zgetenv("OLDPWD");
+    if (oldpwd == NULL || oldpwd[0] == '\0')
+        oldpwd = ztrdup(pwd);  /* initialize `OLDPWD' = `PWD' */

     inittyptab();     /* initialize the ztypes table */
     initlextabs();    /* initialize lexing tables    */