Skip to content

fix(xonsh): Ignore empty $ATUIN_HISTORY_ID in postcommand - #3562

Open
nahoj wants to merge 1 commit into
atuinsh:mainfrom
nahoj:empty-history-id
Open

fix(xonsh): Ignore empty $ATUIN_HISTORY_ID in postcommand#3562
nahoj wants to merge 1 commit into
atuinsh:mainfrom
nahoj:empty-history-id

Conversation

@nahoj

@nahoj nahoj commented Jul 5, 2026

Copy link
Copy Markdown

This PR makes xonsh behavior consistent with other shells: atuin history end should ignore $ATUIN_HISTORY_ID being undefined OR empty.

$ATUIN_HISTORY_ID can be empty if atuin history start fails, which happened to me in #3561.

Checks

  • I am happy for maintainers to push small adjustments to this PR, to speed up the review cycle
  • I have checked that there are no existing pull requests for the same thing

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Fossier: Manual Review Requested

@nahoj is a new contributor. A maintainer should review this PR before merging.

Score Breakdown

Total Score: 53.2/100 | Confidence: 100% | Outcome: REVIEW

Signal Value Score Weight
account_age 5721 1.00 0.09
public_repos 44 1.00 0.05
contribution_history 46 0.23 0.05
follower_ratio 5.0 1.00 0.05
bot_signals False 0.50 0.04
open_prs_elsewhere 13 0.00 0.09
closed_prs_elsewhere 13 0.00 0.10
merged_prs_elsewhere 34 1.00 0.08
prior_interaction 0 0.00 0.08
activity_velocity 2 0.67 0.08
pr_content ... 0.70 0.08
commit_email no_email 0.50 0.04
pr_description ... 0.70 0.05
repo_stars 30449 0.30 0.04
org_membership 0 0.20 0.03
commit_verification ... 1.00 0.04
contributor_stars 30 0.60 0.04

@greptile-apps

greptile-apps Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes the xonsh postcommand hook to skip atuin history end when $ATUIN_HISTORY_ID is empty, not just when it's undefined. The precommand already sets it to "" on failure (line 19), so the old not in check was insufficient.

  • Changes "ATUIN_HISTORY_ID" not in ${...} to not ${...}.get("ATUIN_HISTORY_ID"), which treats both missing and empty-string values as falsy — aligning xonsh with the behavior of other supported shells.

Confidence Score: 5/5

Single-line guard change with no side effects; straightforward and correct.

The fix directly addresses the code path where $ATUIN_HISTORY_ID is set to "" on atuin history start failure, and using .get() with a falsy check handles both the missing-key and empty-string cases correctly.

No files require special attention.

Important Files Changed

Filename Overview
crates/atuin/src/shell/atuin.xsh Guard in postcommand now skips execution when ATUIN_HISTORY_ID is missing or empty, consistent with precommand setting it to "" on failure.

Reviews (1): Last reviewed commit: "fix(xonsh): Ignore empty $ATUIN_HISTORY_..." | Re-trigger Greptile

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant