Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for oh-my-zsh plugin. #101

Merged
merged 5 commits into from
Oct 2, 2019
Merged

Conversation

alxbl
Copy link
Contributor

@alxbl alxbl commented Oct 1, 2019

Thanks for making this project, it's really cool! I'd been looking for something like this for a while.
I noticed that your shell widget is pretty much stand-alone and I wanted to be able to use navi directly as an oh-my-zsh plugin, so I added a few things to the widget and moved it to the root of the repository so that the repo can be cloned straight into the plugins folder and detected when added in the plugin array.

I think it should be similarly simple with fish, but I'm not too familiar with that shell, so I'll leave it to someone else. The navi.plugin.zsh file should still be sourcable as a widget for people not using oh-my-zsh, so technically the shell/ directory could be removed and navi widgetupdated to use the new file.

What's included

Future Work

  • Pre-filter using $BUFFER when hitting Alt+G to get something similar to navi query
  • Update navi widget zsh to use navi.plugin.zsh

Let me know what you think. I'm working on a few cheatsheets, and will open PRs for those as they become ready.

Cheers,
Alex

@alxbl alxbl requested a review from denisidoro as a code owner October 1, 2019 14:56
@welcome
Copy link

welcome bot commented Oct 1, 2019

Thanks for opening this pull request!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@denisidoro denisidoro mentioned this pull request Oct 1, 2019
@denisidoro
Copy link
Owner

@alxbl, I've created a PR with more changes: #103
Could you check that, please?
(I don't know how to add commits to this PR, given that I don't have write access to your fork 😅)

@alxbl
Copy link
Contributor Author

alxbl commented Oct 1, 2019

Your changes looks good to me, if you'd like, I can merge them into my PR, or you can merge your PR after this one? I'll push the changes you requested.

@denisidoro
Copy link
Owner

I'd rather you merge my changes into your PR so that master will always stay up to date

@alxbl
Copy link
Contributor Author

alxbl commented Oct 1, 2019

I've merged all of your changes into the branch. The last thing I'd like to discuss before merging is the OMZ upgrade path:

I think it would make more sense as cd "$ZSH_CONFIG/plugins/navi" && git pull considering that sudo make update will implicitly call make install, which results in symlinks in /usr/bin and partly defeats the point of having it as a plugin. Let me know what you think.

@alxbl
Copy link
Contributor Author

alxbl commented Oct 1, 2019

Made the fix from the discussion in #103. Should be all set if you want to take a last look before merging.

Cheers!
Alex

@denisidoro denisidoro merged commit d9a7486 into denisidoro:master Oct 2, 2019
@welcome
Copy link

welcome bot commented Oct 2, 2019

Congrats on merging your first pull request!

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.

None yet

2 participants