Skip to content

Conversation

akinomyoga
Copy link
Collaborator

Commit b6de491 adds a check for the current working directory. Checking PWD isn't sufficient because PWD can be different from the actual current working directory.

Then, the second commit fixes the issue reported at #1426. command cd -- "$_original_pwd" was missing for the early return with the completion error by builtin compgen.

Fixes #1426.

Copy link
Owner

@scop scop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a valid scenario where $PWD would be changed? I can see that it could be, but then again so could one modify the pwd command to do something that we don't expect.

I'm generally not quite comfortable in trying to cater for everything possible one might want do to break things, so I'd rather see us only addressing scenarios we know are "valid" and exist in the wild, but I don't feel strongly enough about this particular case to request changes.

Anyway the substance of the PR LGTM, thanks!

@akinomyoga
Copy link
Collaborator Author

akinomyoga commented Sep 18, 2025

Is there a valid scenario where $PWD would be changed? I can see that it could be, but then again so could one modify the pwd command to do something that we don't expect.

#1426 has been a case where local PWD=${PWD-} was used in _comp_compgen__call_builtin, but I forgot to return to the original directory on a error. This could have been detected earlier if the actual current working directory $(pwd) had been tested. Is it an invalid case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Completion can change working directory
2 participants