Skip to content

Conversation

@lmBored
Copy link

@lmBored lmBored commented Jan 14, 2026

This is a "continuation" of #1177 - which was a draft that used FIFOs (named pipes) to get output from the popup, however this causes popup not being closed properly, so in this PR I use tmpfile to store the result and read after popup closes. @ellie could you review this PR please?

P.S. Thank you @immae for sharing your idea!

Feature

  • Option to use tmux popup window in config.toml
  • Customize window width/height in config.toml
  • Tmux display-popup for zsh, bash, fish

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

@ellie
Copy link
Member

ellie commented Jan 14, 2026

@ellie could you review this PR please?

sure, but please at least wait until CI passes/finishes lol 😂

@greptile-apps
Copy link

greptile-apps bot commented Jan 14, 2026

Greptile Summary

  • Implements tmux display-popup support for atuin history search with configurable window dimensions and fallback behavior
  • Adds new [tmux] configuration section with enabled, width, and height settings defaulting to enabled
  • Updates shell integration scripts (atuin.bash, atuin.fish, atuin.zsh) to use temporary files instead of FIFOs for popup communication

Important Files Changed

Filename Overview
crates/atuin-client/src/settings.rs Adds new Tmux struct with enabled default; potential breaking change for existing users
crates/atuin/src/shell/atuin.bash Implements tmux popup with shell command construction that may have injection risks
crates/atuin/src/shell/atuin.fish Adds tmux popup functionality with version checking; potential regex parsing edge cases
crates/atuin/src/shell/atuin.zsh Implements popup support with temporary file handling and cleanup mechanisms

Confidence Score: 3/5 - While this is a solid feature implementation, there are potential security concerns around shell command construction, default configuration changes that could surprise users, and some edge cases in version parsing that need attention.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

11 files reviewed, 7 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +87 to +91
set -l query (commandline -b | string replace -a "'" "'\\''")
set -l escaped_args ""
for arg in $argv
set escaped_args "$escaped_args '"(string replace -a "'" "'\\''" -- $arg)"'"
end
Copy link

Choose a reason for hiding this comment

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

logic: Shell injection vulnerability: query and args are constructed into shell command without proper escaping validation. The escaping logic may not handle all edge cases.

Copy link
Author

Choose a reason for hiding this comment

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

This is likely a false positive. Single quotes are POSIX strong quote, and as variables are wrapped in single quotes within sh -c, replacing every internal single quote with '\'' is sufficient to render all other (special) characters

@lmBored
Copy link
Author

lmBored commented Jan 14, 2026

sure, but please at least wait until CI passes/finishes lol 😂

Oopsie sorry, I forgot about CI... XD

@lmBored lmBored changed the title Option to use tmux display-popup with atuin feat: add option to use tmux display-popup Jan 14, 2026
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.

2 participants