Skip to content

Conversation

@moviuro
Copy link
Contributor

@moviuro moviuro commented Jul 25, 2022

We have to account for a lot of possibilities (see zshbuiltins(1)): we stay on the safe side and only change cd target if we're sure about what we do. See examples:

% zsh -d -f             # reset the shell
% cd
% touch -- -q
% cd -q                 # should do nothing; 43090cf version behaves weirdly here, replacing -q with .
% cd /home
% cd /home /etc         # should go to /etc (replace home with etc in PWD)
/etc
% cd /home /etc         # this will fail [1]
cd: string not in pwd: /home
% cd /home
% cd -q /home /etc      # should go to /etc
/etc
% cd -q /home /etc      # this will fail [2]
cd: string not in pwd: /home
% cd
% touch +123456
% cd +123456            # this will fail [3]; 43090cf version behaves weirdly here, replacing +123456 with .

[1]   I chose to make this one work. If `cd old new` fails, then try to
      `cd old`
[2,3] I chose to fail too in that case. Handling cd arguments is not what
      I was planning to do when I started working on this patch ;P

We have to account for a lot of possibilities (see zshbuiltins(1)): we
stay on the safe side and only change cd target if we're sure about what
we do. See examples:

% zsh -d -f             # reset the shell
% cd
% touch -- -q
% cd -q                 # should do nothing; 43090cf version behaves weirdly here, replacing -q with .
% cd /home
% cd /home /etc         # should go to /etc (replace home with etc in PWD)
/etc
% cd /home /etc         # this will fail [1]
cd: string not in pwd: /home
% cd /home
% cd -q /home /etc      # should go to /etc
/etc
% cd -q /home /etc      # this will fail [2]
cd: string not in pwd: /home
% cd
% touch +123456
% cd +123456            # this will fail [3]; 43090cf version behaves weirdly here, replacing +123456 with .

[1]   I chose to make this one work. If `cd old new` fails, then try to
      `cd old`
[2,3] I chose to fail too in that case. Handling cd arguments is not what
      I was planning to do when I started working on this patch ;P
@mika
Copy link
Member

mika commented Jul 25, 2022

Thanks, @moviuro - any objections against merging this as-is from your side, @ft?

@moviuro
Copy link
Contributor Author

moviuro commented Jul 25, 2022

You probably want to run some more tests on it. I ran a few and made sure my usecases were covered, though I might have missed some.

Even got to learn about zshbuiltins(1) in the process!

ImOdd

This comment was marked as spam.

@zeha zeha requested a review from ft December 10, 2024 17:03
@mika
Copy link
Member

mika commented May 14, 2025

@ft maybe you could please take a look at this once you've some spare minutes? 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

3 participants