[PATCH] cd: Append 2nd argument to CWD if 1st is empty

[PATCH] cd: Append 2nd argument to CWD if 1st is empty

[Julian Prein]

[-- Attachment #1.1: Type: text/plain, Size: 1948 bytes --]

Hello,

Since this could break some scripts relying on that behavior, the change could
also be put behind an option that is off by default. I first wanted to hear what
the general opinion on this change is, but would happily send a v2, if
requested.

Thanks,
Julian


From fdc9b7401d10d37a927394e782071af933469a20 Mon Sep 17 00:00:00 2001
From: Julian Prein <druckdev@protonmail.com>
Date: Thu, 30 Nov 2023 19:12:33 +0100
Subject: [PATCH] cd: Append 2nd argument to CWD if 1st is empty

In cd's second form:

    cd [ -qsLP ] old new

Currently when old is empty, cd will prepend new to the name of the
current directory. As this has very limited use cases (e.g.
`cd '' /proc` when in `/sys/...`), change the behaviour of cd to
**append** new instead.
---
 Doc/Zsh/builtins.yo | 3 ++-
 Src/builtin.c       | 2 ++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index 8f310f6cf591..8694298aaeac 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -287,7 +287,8 @@ directory hash table.
 

 The second form of tt(cd) substitutes the string var(new)
 for the string var(old) in the name of the current directory,
-and tries to change to this new directory.
+and tries to change to this new directory. If var(old) is empty, var(new)
+is appended to the name of the current directory.
 

 The third form of tt(cd) extracts an entry from the directory
 stack, and changes to that directory.  An argument of the form
diff --git a/Src/builtin.c b/Src/builtin.c
index 9e08a1dbce99..dad9ff745a5c 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -921,6 +921,8 @@ cd_get_dest(char *nam, char **argv, int hard, int func)
 	}
 	len1 = strlen(argv[0]);
 	len2 = strlen(argv[1]);
+	if (!len1)
+	    u = pwd + strlen(pwd);
 	len3 = u - pwd;
 	d = (char *)zalloc(len3 + len2 + strlen(u + len1) + 1);
 	strncpy(d, pwd, len3);
-- 

2.42.1

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

Re: [PATCH] cd: Append 2nd argument to CWD if 1st is empty

[Roman Perepelitsa]

On Fri, Dec 1, 2023 at 4:12 AM Julian Prein <druckdev@protonmail.com> wrote:
>
> Currently when old is empty, cd will prepend new to the name of the
> current directory. As this has very limited use cases (e.g.
> `cd '' /proc` when in `/sys/...`), change the behaviour of cd to
> **append** new instead.

What is the use case of the new behavior?

Roman.

Re: [PATCH] cd: Append 2nd argument to CWD if 1st is empty

[Julian Prein]

[-- Attachment #1.1: Type: text/plain, Size: 949 bytes --]

On Friday, December 1st, 2023 at 10:21, Roman Perepelitsa <roman.perepelitsa@gmail.com> wrote:
>
> What is the use case of the new behavior?

I often work in directories that are named the same with one having an extra
suffix. Examples would be ./dotfiles and ./dotfiles-private, or ./repo and
./repo-branchname where repo-branchname is a linked git-worktree of repo to work
in different branches simultaneously.
To switch from repo-branchname to repo, I can type `cd -branchname ''`. The
inverse does currently not work though; I'd have to type
`cd ../repo-branchname`.

My motivation to submit this patch was primarily the assumption that specifying
an empty argument is an edge case that is not well defined in the documentation
and that the current behaviour is limited in its use cases. To me an appendix
seems more flexible in the ways a user can utilize this. But this might very
well be biased by my usage.

Thanks,
Julian

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

Re: [PATCH] cd: Append 2nd argument to CWD if 1st is empty

[Mikael Magnusson]

On 12/1/23, Julian Prein <druckdev@protonmail.com> wrote:
> On Friday, December 1st, 2023 at 10:21, Roman Perepelitsa
> <roman.perepelitsa@gmail.com> wrote:
>>
>> What is the use case of the new behavior?
>
> I often work in directories that are named the same with one having an
> extra
> suffix. Examples would be ./dotfiles and ./dotfiles-private, or ./repo and
> ./repo-branchname where repo-branchname is a linked git-worktree of repo to
> work
> in different branches simultaneously.
> To switch from repo-branchname to repo, I can type `cd -branchname ''`. The
> inverse does currently not work though; I'd have to type
> `cd ../repo-branchname`.

I'm not opposed to the patch, but you could just cd $PWD-branchname in
this case. It's also easy to get the desired functionality with a
wrapper function (admittedly slightly less easy if you also want to
handle edge cases like cd -L '' foo but you probably don't):
cd() {
  if [[ $# == 2 ]] && [[ $1 = "" ]]; then
    builtin cd $PWD$2
  else
    builtin cd "$@"
  fi
}

-- 
Mikael Magnusson

Re: [PATCH] cd: Append 2nd argument to CWD if 1st is empty

[Julian Prein]

[-- Attachment #1.1: Type: text/plain, Size: 852 bytes --]

On Friday, December 1st, 2023 at 14:51, Mikael Magnusson <mikachu@gmail.com> wrote:
> 

> I'm not opposed to the patch, but you could just cd $PWD-branchname in
> this case.

True. For me personally though, typing $PWD always feels a bit clunky and
unergonomic as I have to hold shift constantly.

> It's also easy to get the desired functionality with a
> wrapper function (admittedly slightly less easy if you also want to
> handle edge cases like cd -L '' foo but you probably don't):
> cd() {
>   if [[ $# == 2 ]] && [[ $1 = "" ]]; then
>     builtin cd $PWD$2
>   else
>     builtin cd "$@"
>   fi
> }

Yes, that is probably what I'll do, if the patch gets rejected. But I am not a
big fan of wrapper functions that have to parse arguments and tend to not want
to break existing functionality through them.

Thanks,
Julian

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

Re: [PATCH] cd: Append 2nd argument to CWD if 1st is empty

[Lawrence Velázquez]

On Fri, Dec 1, 2023, at 8:32 AM, Julian Prein wrote:
> My motivation to submit this patch was primarily the assumption that specifying
> an empty argument is an edge case that is not well defined in the documentation
> and that the current behaviour is limited in its use cases. To me an appendix
> seems more flexible in the ways a user can utilize this.

FWIW the current behavior -- while undocumented -- is aligned with
that of ksh, and this change would break that alignment and become
one more thing for ksh emulation to handle.

	% cd /bin
	% ksh -c 'print "$KSH_VERSION"; cd "" /usr'
	Version AJM 93u+ 2012-08-01
	/usr/bin
	% mksh -c 'print "$KSH_VERSION"; cd "" /usr'
	@(#)MIRBSD KSH R59 2020/10/31
	/usr/bin

The old behavior is not a bug, and the new behavior doesn't strike
me as particularly useful, so it's not clear to me that this would
be worth it (either as a breaking change or one hidden behind yet
another option).

-- 
vq

Re: [PATCH] cd: Append 2nd argument to CWD if 1st is empty

[Roman Perepelitsa]

On Fri, Dec 1, 2023 at 7:23 PM Lawrence Velázquez <larryv@zsh.org> wrote:
>
> On Fri, Dec 1, 2023, at 8:32 AM, Julian Prein wrote:
> > My motivation to submit this patch was primarily the assumption that specifying
> > an empty argument is an edge case that is not well defined in the documentation
> > and that the current behaviour is limited in its use cases. To me an appendix
> > seems more flexible in the ways a user can utilize this.
>
> FWIW the current behavior -- while undocumented -- is aligned with
> that of ksh, and this change would break that alignment and become
> one more thing for ksh emulation to handle.

What I like about the current behavior is that it does not require
additional documentation. The existing documentation is sufficient.
What happens when the first argument is empty is consistent with what
happens when it's not: the first matching substring is replaced, which
in the case of an empty substring means the beginning of the string.
The patch changes a rare case that I don't personally care about, but
it also adds a special case, and I do care about special cases in
general -- I do not like them.

Roman.