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.

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

Add PWD to the Reedline state #796

Merged
merged 2 commits into from
Jul 5, 2024
Merged

Add PWD to the Reedline state #796

merged 2 commits into from
Jul 5, 2024

Conversation

YizhePKU
Copy link
Contributor

This PR adds the current working directory as part of the Reedline state, instead of using std::env::current_dir() to retrieve it from the environment. Applications should call Reedline::with_cwd() to update the current working directory when it changes.

This is necessary to support nushell/nushell#12922, which causes std::env::current_dir() to deviate from Nushell's $env.PWD.

@YizhePKU YizhePKU mentioned this pull request May 31, 2024
Merged
@IanManske
Copy link
Member

Currently, this sets cwd to an empty string in all cases, unless explicitly overwritten. Won't this break existing code that was relying on the std::env::current_dir behavior?

@devyn
Copy link
Contributor

devyn commented Jun 13, 2024

It would probably be best that it's either required or defaults to the actual current dir. Reedline is intended to be useful for more than just nushell

devyn
devyn requested changes Jun 14, 2024
View reviewed changes
src/engine.rs Outdated
@@ -224,6 +228,7 @@ impl Reedline {
hide_hints: false,
validator,
use_ansi_coloring: true,
cwd: "".to_string(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make it clear - I don't think an empty string should be the default. I think making it an Option and then grabbing process working directory if it's None is probably the best way to go

@YizhePKU
Copy link
Contributor Author

Just to make it clear - I don't think an empty string should be the default. I think making it an Option and then grabbing process working directory if it's None is probably the best way to go.

So you're saying that if cwd is set to None, we fallback to the old behavior (calling std::env::current_dir()), so that we don't break existing code that doesn't require special PWD logic?

That sounds good. I'll work on it.

@devyn
Copy link
Contributor

devyn commented Jun 19, 2024

Yeah, exactly. Thanks!

@fdncred
Copy link
Collaborator

fdncred commented Jun 29, 2024 •
edited
Loading

@YizhePKU Do you have any more time to work on this in regards to @devyn's comments? I'd like to get this landed asap.

@YizhePKU
Copy link
Contributor Author

YizhePKU commented Jul 3, 2024

I think this will do? I'll update the other PR if this gets merged.

@fdncred
Copy link
Collaborator

fdncred commented Jul 3, 2024

Thanks @YizhePKU! We appreciate you coming back around to this PR. @devyn, when you get a chance can you please look at this. I'd like to get it landed if possible.

@devyn
Copy link
Contributor

devyn commented Jul 5, 2024

Looks correct to me. Thanks!

@devyn devyn merged commit bde962b into nushell:main Jul 5, 2024
6 checks passed
@YizhePKU YizhePKU deleted the custom-cwd branch July 7, 2024 15:13
fdncred pushed a commit to nushell/nushell that referenced this pull request Jul 7, 2024
@YizhePKU
Fix PWD-aware command hints (#13024)
152fb5b 
This PR fixes PWD-aware command hints by sending PWD to the Reedline
state in every REPL loop. This PR should be merged along with
nushell/reedline#796.

Fixes #12951.
leftwo referenced this pull request in oxidecomputer/crucible Aug 17, 2024
@renovate
Update Rust crate reedline to 0.33.0 (#1417)
3cb3c2a 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [reedline](https://togithub.com/nushell/reedline) |
workspace.dependencies | minor | `0.30.0` -> `0.33.0` |

---

### Release Notes

<details>
<summary>nushell/reedline (reedline)</summary>

###
[`v0.33.0`](https://togithub.com/nushell/reedline/releases/tag/v0.33.0)

[Compare
Source](https://togithub.com/nushell/reedline/compare/v0.32.0...v0.33.0)

#### What's Changed

- fix some new clippy warnings by
[@&#8203;fdncred](https://togithub.com/fdncred) in
[https://github.com/nushell/reedline/pull/790](https://togithub.com/nushell/reedline/pull/790)
- Add PWD to the `Reedline` state by
[@&#8203;YizhePKU](https://togithub.com/YizhePKU) in
[https://github.com/nushell/reedline/pull/796](https://togithub.com/nushell/reedline/pull/796)
- Fix [#&#8203;793](https://togithub.com/nushell/reedline/issues/793)
using width() for column menu alignements with special char by
[@&#8203;Jiogo18](https://togithub.com/Jiogo18) in
[https://github.com/nushell/reedline/pull/794](https://togithub.com/nushell/reedline/pull/794)
- Make menus process events before updating working details by
[@&#8203;ysthakur](https://togithub.com/ysthakur) in
[https://github.com/nushell/reedline/pull/799](https://togithub.com/nushell/reedline/pull/799)
- Feature: vi visual mode by
[@&#8203;adamschmalhofer](https://togithub.com/adamschmalhofer) in
[https://github.com/nushell/reedline/pull/800](https://togithub.com/nushell/reedline/pull/800)

#### New Contributors

- [@&#8203;YizhePKU](https://togithub.com/YizhePKU) made their first
contribution in
[https://github.com/nushell/reedline/pull/796](https://togithub.com/nushell/reedline/pull/796)
- [@&#8203;Jiogo18](https://togithub.com/Jiogo18) made their first
contribution in
[https://github.com/nushell/reedline/pull/794](https://togithub.com/nushell/reedline/pull/794)
- [@&#8203;adamschmalhofer](https://togithub.com/adamschmalhofer) made
their first contribution in
[https://github.com/nushell/reedline/pull/800](https://togithub.com/nushell/reedline/pull/800)

**Full Changelog**:
nushell/reedline@v0.32.0...v0.33.0

###
[`v0.32.0`](https://togithub.com/nushell/reedline/releases/tag/v0.32.0):
0.32.0

[Compare
Source](https://togithub.com/nushell/reedline/compare/v0.31.0...v0.32.0)

#### What's Changed

- add bashism `!term` to prefix search for last command beginning with
`term` by [@&#8203;fdncred](https://togithub.com/fdncred) in
[https://github.com/nushell/reedline/pull/779](https://togithub.com/nushell/reedline/pull/779)
- Remove debug print by
[@&#8203;sholderbach](https://togithub.com/sholderbach) in
[https://github.com/nushell/reedline/pull/784](https://togithub.com/nushell/reedline/pull/784)
- fix ide menu not reporting correct required_lines by
[@&#8203;maxomatic458](https://togithub.com/maxomatic458) in
[https://github.com/nushell/reedline/pull/781](https://togithub.com/nushell/reedline/pull/781)
- Fix (properly) the logic around prompt re-use & Host Command handling
by [@&#8203;bew](https://togithub.com/bew) in
[https://github.com/nushell/reedline/pull/770](https://togithub.com/nushell/reedline/pull/770)
- fix: unexpected spaces after large buffer input by
[@&#8203;sigoden](https://togithub.com/sigoden) in
[https://github.com/nushell/reedline/pull/783](https://togithub.com/nushell/reedline/pull/783)
- Bump version for `0.32.0` release by
[@&#8203;devyn](https://togithub.com/devyn) in
[https://github.com/nushell/reedline/pull/785](https://togithub.com/nushell/reedline/pull/785)

#### New Contributors

- [@&#8203;bew](https://togithub.com/bew) made their first contribution
in
[https://github.com/nushell/reedline/pull/770](https://togithub.com/nushell/reedline/pull/770)
- [@&#8203;sigoden](https://togithub.com/sigoden) made their first
contribution in
[https://github.com/nushell/reedline/pull/783](https://togithub.com/nushell/reedline/pull/783)
- [@&#8203;devyn](https://togithub.com/devyn) made their first
contribution in
[https://github.com/nushell/reedline/pull/785](https://togithub.com/nushell/reedline/pull/785)

**Full Changelog**:
nushell/reedline@v0.31.0...v0.32.0

###
[`v0.31.0`](https://togithub.com/nushell/reedline/releases/tag/v0.31.0):
0.31.0

[Compare
Source](https://togithub.com/nushell/reedline/compare/v0.30.0...v0.31.0)

New release for [Nushell](https://togithub.com/nushell/nushell) `0.92.0`

#### What's Changed

- Bump version of `strum`/`strum_macros` by
[@&#8203;sholderbach](https://togithub.com/sholderbach) in
[https://github.com/nushell/reedline/pull/768](https://togithub.com/nushell/reedline/pull/768)
- Use the OS clipboard only for explicit cut/copy/paste operations by
[@&#8203;Tastaturtaste](https://togithub.com/Tastaturtaste) in
[https://github.com/nushell/reedline/pull/761](https://togithub.com/nushell/reedline/pull/761)
- Revert "Move left when exiting insert mode" by
[@&#8203;fdncred](https://togithub.com/fdncred) in
[https://github.com/nushell/reedline/pull/773](https://togithub.com/nushell/reedline/pull/773)
- Fix `OpenOptions` clippy by
[@&#8203;sholderbach](https://togithub.com/sholderbach) in
[https://github.com/nushell/reedline/pull/776](https://togithub.com/nushell/reedline/pull/776)
- Bump `fd-lock` requirement and locked deps by
[@&#8203;sholderbach](https://togithub.com/sholderbach) in
[https://github.com/nushell/reedline/pull/775](https://togithub.com/nushell/reedline/pull/775)
- Fix case-consistency searching sqlite history by
[@&#8203;sholderbach](https://togithub.com/sholderbach) in
[https://github.com/nushell/reedline/pull/777](https://togithub.com/nushell/reedline/pull/777)
- Bump version for `0.31.0` release by
[@&#8203;sholderbach](https://togithub.com/sholderbach) in
[https://github.com/nushell/reedline/pull/780](https://togithub.com/nushell/reedline/pull/780)

**Full Changelog**:
nushell/reedline@v0.30.0...v0.31.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 8pm,before 6am" in timezone
America/Los_Angeles, Automerge - "after 8pm,before 6am" in timezone
America/Los_Angeles.

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/oxidecomputer/crucible).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yNi4xIiwidXBkYXRlZEluVmVyIjoiMzguMjYuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@jakeswenson
Copy link

Does anyone know how this relates to nushell/nushell/issues/5799? Not sure if this directly resolves it or if there is more work to be done?

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

@devyn devyn devyn 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.
5 participants
@YizhePKU @IanManske @devyn @fdncred @jakeswenson