Skip to content

Conversation

@matlads
Copy link
Contributor

@matlads matlads commented Aug 7, 2025

User description

This PR adds completion for the jump command, and updates the plugin documentation appropriately


PR Type

Enhancement


Description

  • Add bash completion support for jump command

  • Update jump plugin documentation with installation steps

  • Include usage instructions and completion configuration


Diagram Walkthrough

flowchart LR
  A["jump command"] --> B["completion script"]
  B --> C["auto-complete directories"]
  D["README update"] --> E["installation guide"]
  E --> F["usage instructions"]
Loading

File Walkthrough

Relevant files
Enhancement
jump.completion.sh
Add bash completion for jump                                                         

completions/jump.completion.sh

  • Create new completion script for jump command
  • Extract available jump commands dynamically
  • Implement bash completion function with compgen
  • Register completion function for jump command
+10/-0   
Documentation
README.md
Update jump plugin documentation                                                 

plugins/jump/README.md

  • Add detailed installation instructions with numbered steps
  • Include completion configuration in bashrc setup
  • Add usage section with link to official documentation
  • Improve formatting and structure of documentation
+21/-6   

@qodo-free-for-open-source-projects

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Error Handling

The completion script captures stderr output from jump command but doesn't handle potential parsing failures. If jump command fails or produces unexpected output format, the grep/awk pipeline could fail silently or produce incorrect completions.

local JUMP_COMMANDS=$(jump 2>&1 | command grep -E '^ +' | awk '{print $1}')
COMPREPLY=( $(compgen -W "$JUMP_COMMANDS" -- "${COMP_WORDS[1]}") )
Performance Concern

The completion function executes jump command on every tab completion attempt, which could be slow for large jump databases. Consider caching the results or implementing a more efficient completion strategy.

local JUMP_COMMANDS=$(jump 2>&1 | command grep -E '^ +' | awk '{print $1}')
COMPREPLY=( $(compgen -W "$JUMP_COMMANDS" -- "${COMP_WORDS[1]}") )

@qodo-free-for-open-source-projects
Copy link

qodo-free-for-open-source-projects bot commented Aug 7, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix completion word index logic

Using hardcoded index ${COMP_WORDS[1]} assumes completion is always for the
first argument, but jump may have subcommands or flags. Use
${COMP_WORDS[COMP_CWORD]} to complete the current word being typed.

completions/jump.completion.sh [7]

-COMPREPLY=( $(compgen -W "$JUMP_COMMANDS" -- "${COMP_WORDS[1]}") )
+COMPREPLY=( $(compgen -W "$JUMP_COMMANDS" -- "${COMP_WORDS[COMP_CWORD]}") )
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly proposes using the standard COMP_CWORD variable instead of a hardcoded index, making the completion logic more robust and idiomatic for bash completions.

Medium
General
Add error handling for command extraction

The command extraction logic is fragile and may fail if jump's help output
format changes. Consider using jump's built-in completion support if available,
or add error handling to gracefully handle parsing failures.

completions/jump.completion.sh [6]

-local JUMP_COMMANDS=$(jump 2>&1 | command grep -E '^ +' | awk '{print $1}')
+local JUMP_COMMANDS=$(jump 2>&1 | command grep -E '^ +' | awk '{print $1}' 2>/dev/null || echo "")
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that parsing the jump command's output is fragile and improves robustness by adding error handling to prevent script failure if the output format changes.

Low
  • Update

matlads and others added 4 commits August 10, 2025 01:06
Co-authored-by: Koichi Murase <[email protected]>
This commit updates the plugin's README to indicate that enabling the
completion is an optional part of using the jump plugin.
In this particular case, follow the one concerning functions defined by
a specific plugin.
@matlads matlads changed the title Feature/jump completion feat(completions/jump): add completion for jump Aug 9, 2025
@matlads matlads requested a review from akinomyoga August 9, 2025 22:30
Copy link
Contributor

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

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

I now noticed that the indentation is made with TAB, but I use two spaces in this repository. Could you update the indentation?

@matlads
Copy link
Contributor Author

matlads commented Aug 12, 2025

I now noticed that the indentation is made with TAB, but I use two spaces in this repository. Could you update the indentation?

@akinomyoga fixed.

@akinomyoga akinomyoga merged commit d9a68f9 into ohmybash:master Aug 12, 2025
4 checks passed
@akinomyoga
Copy link
Contributor

@matlads Thanks! I've merged the PR!

@matlads matlads deleted the feature/jump-completion branch August 21, 2025 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants