Skip to content
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.

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

Elminate implicit cd #1764

Closed
wants to merge 1 commit into from

Conversation

krader1961
Copy link
Contributor

Replace the CLI implicit cd behavior with location mode support for literal directory paths.

Fixes #1099

@krader1961 krader1961 force-pushed the issue-1099-implicit-cd branch 2 times, most recently from 9a4fe1c to 1c25a59 Compare February 26, 2024 05:49
@krader1961
Copy link
Contributor Author

Force push to fix the conflict with the change from the 0.20.0-release-notes.md to the 0.21.0-release-notes.md file since this PR was initially created. No other changes were made.

@xiaq
Copy link
Member

xiaq commented Feb 26, 2024

After playing with this a bit, it feels non-ideal:

I now feel literal cd via location mode may not be the best idea. Maybe adding a completer for cd to only show directories and fixing #474 would be a better UX, albeit with slightly more keypresses.

As for the removal of implicit cd, I'd rather deprecate it first and only remove it after the next release.

The headings in tour.md were chosen intentionally to reflect the general functionality rather than Elvish-specific terms, please don't change them.

Thanks for implementing this, and sorry that I've changed my mind. I'll close this PR and add deprecation for implicit cd.

@xiaq xiaq closed this Feb 26, 2024
xiaq added a commit that referenced this pull request Feb 26, 2024
This addresses #1099.

Also link to location mode from the elvdoc of cd (originally part of #1764).
@krader1961 krader1961 deleted the issue-1099-implicit-cd branch February 28, 2024 06:34
@krader1961
Copy link
Contributor Author

I now feel literal cd via location mode may not be the best idea. Maybe adding a completer for cd to only show directories and fixing #474 would be a better UX, albeit with slightly more keypresses.

That is actually what I would prefer, @xiaq. I only did this as an experiment and to gain more experience writing interactive unit tests. I've always maintained that implict cd isn't worth saving just two keystrokes compared to this pull-request. However, if those two keystrokes are worth eliminating it might be worth exploring how to make tab completion work in location mode. Also, the fact my change doesn't handle a ~/ prefix was an oversight and could probably be handled easily.

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.

Keep and document implicit cd? The / (division command) invoked with no args does cd /
2 participants