Skip to content

Fix fip function on zsh#6112

Open
pkwagner wants to merge 1 commit into
basecamp:devfrom
pkwagner:fip-zsh-fix
Open

Fix fip function on zsh#6112
pkwagner wants to merge 1 commit into
basecamp:devfrom
pkwagner:fip-zsh-fix

Conversation

@pkwagner

Copy link
Copy Markdown

The original fip shell function works fine on bash, but not on zsh:

❯ fip remote_host 8081    
Bad local forwarding specification '8081ocalhost:8081'

The reason for this is that zsh interprets the :l part of ssh -f -N -L "$port:localhost as modifier (lowercase), resulting in ssh -f -N -L "${port}ocalhost.

This PR resolves this by using proper ${} template variables, which work on both bash and zsh.

Copilot AI review requested due to automatic review settings June 19, 2026 16:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ Ready to approve

The change is minimal, clearly addresses the described zsh expansion issue, and preserves behavior for bash.

Note: this review does not count toward required approvals for merging.

Pull request overview

This PR updates the fip/dip SSH port-forwarding shell functions to be compatible with zsh by bracing parameter expansions so zsh won’t treat :l (and similar) as a parameter expansion modifier.

Changes:

  • Replace $port:localhost:$port-style expansions with ${port}:localhost:${port} to avoid zsh modifier parsing.
  • Apply the same bracing fix to the pkill pattern used by dip.
  • Minor consistency tweak: brace $1 when assigning host.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File summaries
File Description
default/bash/fns/ssh-port-forwarding Braces variable expansions in fip/dip to prevent zsh from mis-parsing :localhost and breaking ssh -L arguments.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.


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

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