Skip to content

[pull] master from danielrobbins:master - #32

Merged
pull[bot] merged 7 commits into
jynychen:masterfrom
danielrobbins:master
Oct 31, 2025
Merged

pull[bot] merged 7 commits into
jynychen:masterfrom
danielrobbins:master

Conversation

@pull

@pull pull Bot commented Oct 31, 2025 •

Copy link
Copy Markdown

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

Critical fixes for keychain failures when HOME path or username contains spaces:

Core fixes:
- Implemented portable get_owner() function using POSIX-defined ls -ld output
  with intelligent field parsing to distinguish space-in-username from normal
  owner/group fields
- Rewrote write_pidfile() to use robust eval-in-subshell approach for
  extracting variable values from ssh-agent output
- Fixed pidfile generation to properly quote SSH_AUTH_SOCK paths while leaving
  SSH_AGENT_PID unquoted (numeric value)
- All pidfile formats (sh/csh/fish) now correctly handle paths with spaces
- Fixed ssh-agent invocation to always use -s option for Bourne-compatible
  output, simplifying pidfile generation

Testing:
- Added scripts/test-space-home.sh automated test harness that simulates HOME
  directories with spaces and validates proper handling
- Returns proper exit codes for CI integration
- Integrated into GitHub Actions release workflow

Documentation:
- Updated keychain.pod with detailed implementation notes for space handling,
  POSIX compliance, and the robust eval approach
- Added comprehensive COMPATIBILITY section documenting Git Bash/MINGW64 and
  WSL support
- Added ShellCheck disable comments with justification for intentional POSIX
  ls usage

Fixes #188, addresses #185
Fixed sed pattern in Makefile that was incorrectly matching 'br at start of
line without a space, causing it to insert spurious periods in the generated
man page.

Changed pattern from s/^'br/.br/ to s/^'br /.br / to require a space after
'br, which prevents false matches.

Fixes #183
Added comprehensive bash completion for keychain with full --extended mode
support. Based on work by @mikkoi with significant enhancements.

Features:
- Context-aware completion for command-line options, SSH keys, and GPG keys
- Dynamically parses keychain --help for up-to-date option completion
- Completes SSH key names from ~/.ssh/*.pub files
- Completes GPG key IDs (8-character short format)
- Extended mode: sshk:<tab>, gpgk:<tab>, host:<tab> with prefix completion
- Detects hostnames from ~/.ssh/config for host: completion
- Proper handling of colons in completions using __ltrim_colon_completions
- Single-character prefix matching (s<tab> → sshk:)
- Graceful degradation when keychain not in PATH

Installation:
- Added Makefile targets: install-completions and uninstall-completions
- Installs to /usr/share/bash-completion/completions/ by default
- Updated RPM spec to include completion file
- Added README section with installation instructions

Testing:
- Added scripts/test-completion.sh to verify functionality across environments
- Returns proper exit codes (0=success, 1=failure)
- Tests on WSL, Git Bash, and standard Linux shells

Technical improvements:
- Standardized keychain.pod option ordering (short first, then long) for
  regex compatibility
- Added _init_completion -n : to prevent colon word-splitting
- Uses compopt -o nospace to prevent spaces after colons
- ShellCheck compliant

Addresses #186
Updated keychain.spec.in for RHEL 8/9, Fedora, Rocky, AlmaLinux:
- Modernized summary and description (removed obsolete ssh.com/Sun SSH)
- Updated dependencies: sh-utils → coreutils
- Added Recommends: bash-completion
- Added bash completion file installation to RPM
- Installs to standard location: /usr/share/bash-completion/completions/
- Add FUNDING.yml with PayPal (BreezyOps) and GitHub Sponsors options
- Add 'Support This Project' section to README with funding links
@pull pull Bot locked and limited conversation to collaborators Oct 31, 2025
@pull pull Bot added the ⤵️ pull label Oct 31, 2025
@pull
pull Bot merged commit 9566df0 into jynychen:master Oct 31, 2025
1 check passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant