feat: add custom key bindings for exiting vim insert mode in fish shell - #284
Conversation
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Fish shell user experience by integrating a custom key binding for exiting Vim insert mode, making the shell's command line editing more consistent with a Vim workflow. Additionally, it introduces a new alias for the Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdates CI Docker tagging logic in .github/workflows/docker.yml to derive suffix from matrix data, introduce a latest flag, and consolidate tag rules with conditional enablement for push-to-main vs. other events. Adds a Fish shell abbreviation g→git and a new fish_user_key_bindings function to map jj to exit insert mode. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions
participant Meta as Set Metadata
participant Build as Build & Tag
Dev->>GH: Push / PR / Workflow dispatch
GH->>Meta: Run metadata step
Note over Meta: Derive suffix from matrix.build.suffix<br/>Compute "latest" flag (push-to-main?)
Meta-->>Build: Tags + enable flags with suffix
alt Push to main
Build->>Build: Enable short sha tag with suffix
Build->>Build: Enable raw "latest" tag with suffix
Build-->>GH: Publish images with push-to-main tags
else Non-push events
Build->>Build: Enable short sha tag with suffix
Build->>Build: Disable semver/raw patterns as specified
Build->>Build: Optionally enable non-latest raw per flags
Build-->>GH: Publish images with non-push tags
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull Request Overview
This PR adds custom vim-style key bindings to the fish shell configuration, specifically implementing a "jj" key sequence to exit vim insert mode. The changes also include unrelated modifications to shell aliases and Docker workflow configuration.
- Added fish shell function for custom key bindings with "jj" sequence for vim mode navigation
- Added "g" alias for git command in shell configuration
- Updated Docker workflow tagging strategy to handle suffixes and latest tag logic differently
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| home-manager/programs/fish/functions/fish_user_key_bindings.fish | New function implementing "jj" key binding to exit vim insert mode |
| home-manager/programs/fish/default.nix | Added git alias "g" to shell aliases configuration |
| .github/workflows/docker.yml | Updated Docker image tagging logic with suffix handling and conditional latest tag behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| flavor: | | ||
| suffix=${{ github.ref == 'refs/heads/main' && github.event_name == 'push' && matrix.build.suffix || '' }} | ||
| suffix=${{ matrix.build.suffix }} | ||
| latest=${{ github.ref == 'refs/heads/main' && github.event_name == 'push' && 'false' || 'auto' }} |
There was a problem hiding this comment.
The latest tag logic is inverted. When on main branch with push event, it sets latest='false', but the following tags suggest it should be 'true' or 'auto'. This will prevent the latest tag from being applied when expected.
| latest=${{ github.ref == 'refs/heads/main' && github.event_name == 'push' && 'false' || 'auto' }} | |
| latest=${{ github.ref == 'refs/heads/main' && github.event_name == 'push' && 'true' || 'false' }} |
| type=sha,format=short,enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }} | ||
| type=semver,pattern={{version}},enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }} | ||
| type=semver,pattern={{raw}},enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }} | ||
| type=raw,value=latest,enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }} |
There was a problem hiding this comment.
This condition enables the 'latest' tag when NOT on main branch with push event, which contradicts typical Docker tagging conventions where 'latest' should only be applied to main branch releases.
| type=raw,value=latest,enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }} |
There was a problem hiding this comment.
Code Review
This pull request adds a 'g' alias for 'git' and a custom keybinding 'jj' to exit insert mode in the fish shell's vi mode. While the alias is a reasonable personal preference, the implementation of the 'jj' keybinding is incorrect and doesn't function as described in the comment. I've provided a critical review comment with a code suggestion to fix the keybinding, ensuring it correctly exits insert mode and clears the typed characters from the command line.
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name=".github/workflows/docker.yml">
<violation number="1" location=".github/workflows/docker.yml:49">
The new `enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }}` on the semver tag entries disables semver tags during main pushes, so release builds never publish `vX.Y.Z-<arch>` images. The manifest job still expects those tags and will fail to assemble multi-arch semver manifests, leaving release tags missing from GHCR.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| type=sha,format=short,suffix=${{ matrix.build.suffix }},enable=${{ github.ref == 'refs/heads/main' && github.event_name == 'push' }} | ||
| type=raw,value=latest,suffix=${{ matrix.build.suffix }},enable=${{ github.ref == 'refs/heads/main' && github.event_name == 'push' }} | ||
| type=sha,format=short,enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }} | ||
| type=semver,pattern={{version}},enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }} |
There was a problem hiding this comment.
The new enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }} on the semver tag entries disables semver tags during main pushes, so release builds never publish vX.Y.Z-<arch> images. The manifest job still expects those tags and will fail to assemble multi-arch semver manifests, leaving release tags missing from GHCR.
Prompt for AI agents
Address the following comment on .github/workflows/docker.yml at line 49:
<comment>The new `enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }}` on the semver tag entries disables semver tags during main pushes, so release builds never publish `vX.Y.Z-<arch>` images. The manifest job still expects those tags and will fail to assemble multi-arch semver manifests, leaving release tags missing from GHCR.</comment>
<file context>
@@ -40,12 +40,15 @@ jobs:
+ type=sha,format=short,suffix=${{ matrix.build.suffix }},enable=${{ github.ref == 'refs/heads/main' && github.event_name == 'push' }}
+ type=raw,value=latest,suffix=${{ matrix.build.suffix }},enable=${{ github.ref == 'refs/heads/main' && github.event_name == 'push' }}
+ type=sha,format=short,enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }}
+ type=semver,pattern={{version}},enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }}
+ type=semver,pattern={{raw}},enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }}
+ type=raw,value=latest,enable=${{ !(github.ref == 'refs/heads/main' && github.event_name == 'push') }}
</file context>
Summary by cubic
Adds a custom fish key binding to exit vi insert mode with "jj", plus a small alias and improved Docker image tagging logic for matrix builds.
New Features
Bug Fixes