Skip to content

fix(cd): Complete from . on empty CDPATH entry (#1527) #1529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
scop merged 1 commit into from
Jan 2, 2026
Merged

fix(cd): Complete from . on empty CDPATH entry (#1527) #1529

scop merged 1 commit into from
Jan 2, 2026
+13 −0

Conversation

@jmou
Copy link
Contributor

@jmou jmou commented Dec 28, 2025 •
edited by akinomyoga
Loading

Fixes #1527 (edit by akinomyoga)

@jmou
Copy link
Contributor Author

jmou commented Dec 28, 2025

For some reason test_cdable_vars fails on my setup with or without my change, but it seems unrelated.

@jmou jmou mentioned this pull request Dec 28, 2025
Closed
3 tasks
@akinomyoga
Copy link
Collaborator

akinomyoga commented Dec 29, 2025

For some reason test_cdable_vars fails on my setup with or without my change, but it seems unrelated.

Fixes are included as 7395885 and 8899a06 in #1530

scop
scop reviewed Dec 29, 2025
View reviewed changes
test/t/test_cd.py Outdated
assert completion == ["foo1", "foo2"]

@pytest.mark.complete(
"cd ", cwd="shared/empty_dir", env=dict(CDPATH=":../default")
Copy link
Owner

@scop scop Dec 29, 2025

Choose a reason for hiding this comment

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

With this, we're testing we no longer complete anything from / which is good. I think it would be even better if we'd cwd somewhere that has subdirs, in order to test that we actually do offer completions from that dir due to the empty entry?

@jmou
Copy link
Contributor Author

jmou commented Dec 30, 2025

Good point, I switched them around. I don't see any other directories in shared with subdirectories, so it's now only testing completion from the empty CDPATH entry and not the second entry. I'm happy to change the second CDPATH entry to another directory with subdirectories, if someone can suggest an appropriate directory to depend on for this test.

Rebased with the fix for test_cdable_vars

@scop
Copy link
Owner

scop commented Dec 30, 2025

I think we can use for example shared/ as the other dir, no need to make it fancier than that. (It does contain bin/ which is a thing in / too, but I don't think that's a problem.)

@jmou
Copy link
Contributor Author

jmou commented Dec 31, 2025

Updated

scop
scop approved these changes Jan 2, 2026
View reviewed changes
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.

Thanks!

@scop scop merged commit 6f1eba6 into scop:main Jan 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scop scop scop approved these changes

@akinomyoga akinomyoga akinomyoga approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Empty string entry in CDPATH completes from /

3 participants

@jmou @akinomyoga @scop