fix(nu): ignore missing history id in pre-prompt#3537
Conversation
Fossier: Manual Review Requested
Score BreakdownTotal Score: 58.3/100 | Confidence: 100% | Outcome: REVIEW
|
Greptile SummaryAdds One subtlety: Confidence Score: 4/5Safe to merge — minimal one-line defensive fix with no regression risk. Change is correct, matches existing startup code, and fixes a real user-reported error. Minor open question about the overlay scoping edge case that makes both the guard and No files require special attention. Important Files Changed
Reviews (1): Last reviewed commit: "fix(nu): ignore missing history id in pr..." | Re-trigger Greptile |
|
|
||
| } | ||
| hide-env ATUIN_HISTORY_ID | ||
| hide-env -i ATUIN_HISTORY_ID |
There was a problem hiding this comment.
The
hide-env -i addition is correct and matches the startup call on line 15. However, _atuin_pre_prompt already has if 'ATUIN_HISTORY_ID' not-in $env { return } at the top — logically, hide-env should only be reached when the variable is present, so the original should not error under normal scoping.
The fact that it does error (#3520) implies Nushell overlay/subshell environments can make not-in $env and hide-env disagree about variable visibility. Adding -i is the right defensive fix, but it would be useful to document this overlay-scoping edge case in a comment so future maintainers understand why both the guard and -i are needed.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Fixes #3520.
This changes the Nushell pre-prompt hook to use
hide-env -i ATUIN_HISTORY_ID, matching the startup cleanup above it. That keeps overlay/subshell prompts from erroring whenATUIN_HISTORY_IDis already absent.Checks
Verification:
git diff --checknuwas not available in this runner, so I could not run a local Nushell source smoke test.