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

Search Function #314

Merged
merged 7 commits into from
May 11, 2016
Merged

Search Function #314

merged 7 commits into from
May 11, 2016

Conversation

lfiolhais
Copy link
Contributor

This PR adds a search command to OMF per #292. The search command supports searches for both themes and packages. To execute a search run omf search with the flags -t / --theme for themes or -pkg / --package for packages. If neither flag is present both themes and packages will be searched.

The output is as follow:

↪ omf search t
Packages
apt             python
artisan             rustup
await               ssh-term-helper
basename-compat         tab
battery             technicolor
export              termux
extract             thefuck
fonts               tiny
getopts             title
git-flow            tmux-zen
homebrew-command-not-found  weather
kill-on-port

Themes
agnoster        integral        taktoa
batman          jacaetevha      technopagan
bobthefish      mtahmed         toaster
default         numist          tomita
gitstatus       pastfish        trout
godfather       simple-ass-prompt

↪ omf search -t simple
Themes
simple-ass-prompt   simplevi

↪ omf search -pkg nv
Packages
direnv      foreign-env nvm     rbenv

This my first "big" PR to OMF so any feedback is welcomed and appreciated. 😄

__omf.search.usage
end
case '*';
__omf.search.usage
Copy link
Member

Choose a reason for hiding this comment

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

The idea is that all command line interface stuff like parameter should be done in omf.cli.* functions. Please move this "0 arguments" case there 👍

@derekstavis
Copy link
Member

Absolutely loved it! Very cool! I will take a closer look at weekend!

@lfiolhais
Copy link
Contributor Author

lfiolhais commented May 7, 2016

I've updated the code according to your feedback.

@sagebind
Copy link
Member

sagebind commented May 8, 2016

I can test this some more later today, and then let's see this get merged! Great work, @lfiolhais!

@sagebind
Copy link
Member

@lfiolhais I see just one bug with omf.cli.search; other than that, it works excellently. Fix the bug and I think we can merge!

case "-t" "--theme";
omf.search.theme $argv[2]
case '*';
__omf.search.usage
Copy link
Member

Choose a reason for hiding this comment

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

This is a typo I think; it should be __omf.cli.search.usage instead of __omf.search.usage.

@lfiolhais
Copy link
Contributor Author

@CoderStephen Fixed!

@sagebind
Copy link
Member

@lfiolhais Excellent; everything looks good to me. Just rebase onto master and then I can merge. Thanks for the contribution!

You can now search the OMF db for both packages and themes.
To search for a package simply type `omf search -pkg <name-of-pkg>` or `omf search --package <name-of-pkg>`. To search for a theme the flags are `-t` or `--theme`.
If you are not sure about what you are searching running `omf search <item>` will net you results for packages and themes.
@lfiolhais
Copy link
Contributor Author

The error reported by travis is unrelated to my changes.

@sagebind
Copy link
Member

👍

@sagebind sagebind merged commit 15e9af8 into oh-my-fish:master May 11, 2016
@lfiolhais lfiolhais deleted the feature/search branch May 15, 2016 17:40
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

3 participants