Skip to content

Conversation

@xtetsuji
Copy link
Owner

@xtetsuji xtetsuji commented Nov 6, 2025

No description provided.

@xtetsuji xtetsuji requested a review from Copilot November 6, 2025 12:00
@xtetsuji xtetsuji self-assigned this Nov 6, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds conditional shell integration support for the Kiro terminal emulator, following the established pattern used for other development tools in the repository.

  • Adds Kiro terminal integration that sources shell-specific integration scripts when running in the Kiro terminal
  • Uses the existing exists, shell_name, and source_if_readable utility functions
  • Follows the same structure as direnv integration (lines 127-134)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +147 to +153
if [ -n "$shell" ]; then
kiro_integration_path="$(kiro --locate-shell-integration-path "$shell" 2>/dev/null)"
if [ -n "$kiro_integration_path" ]; then
source_if_readable "$kiro_integration_path"
fi
unset kiro_integration_path
fi
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

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

The conditional check for shell_name on line 147 is unnecessary. Unlike the zoxide and mise integrations (lines 196-215) that verify shell_name exists as a function with exists shell_name, the direnv integration (lines 128-134) directly uses shell_name without this check. Since shell_name is defined in path_functions which is sourced before common_env, and returns an empty string for unsupported shells, the inner check is redundant. If the empty string is a concern, it would be caught by the subsequent validation on line 149. Consider simplifying to match the direnv pattern or add the exists shell_name check in the outer condition to match zoxide/mise.

Suggested change
if [ -n "$shell" ]; then
kiro_integration_path="$(kiro --locate-shell-integration-path "$shell" 2>/dev/null)"
if [ -n "$kiro_integration_path" ]; then
source_if_readable "$kiro_integration_path"
fi
unset kiro_integration_path
fi
kiro_integration_path="$(kiro --locate-shell-integration-path "$shell" 2>/dev/null)"
if [ -n "$kiro_integration_path" ]; then
source_if_readable "$kiro_integration_path"
fi
unset kiro_integration_path

Copilot uses AI. Check for mistakes.
@xtetsuji
Copy link
Owner Author

duplicated? #52

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