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

Simplify and fix right prompt #20

Merged
merged 2 commits into from Sep 22, 2016
Merged

Simplify and fix right prompt #20

merged 2 commits into from Sep 22, 2016

Conversation

olbrew
Copy link
Contributor

@olbrew olbrew commented Jun 16, 2016

The right prompt shows a Vi mode indicator.
This commit rewrites it with native fish-shell functions, fixes the coloring
issue seen before and adds a new check for the vi mode used from fish v2.3
onward so it is only shown when vi mode is enabled.

This is the follow up pull request as discussed in #19

@olbrew
Copy link
Contributor Author

olbrew commented Aug 7, 2016

@sn0cr can you merge this?
Worth noting that the newest stable 2.3.1 fixes an issue with the mode_prompt and therefore it now has to be manually disabled which can easily be done by adding the following to your fish.config:

# Remove builtin vi mode indicator in favour of custom theme
function fish_mode_prompt
end function

I've also added this caveat to the README.

@derekstavis
Copy link
Member

Hey @olbrew I will have a look at your pull request and merge if it works for me. Thanks for the ping.

@@ -26,7 +27,6 @@ For Mac users, I highly recommend iTerm 2 + Solarized Dark
* Current virtualenv (Python)
You will probably want to disable the default virtualenv prompt. Add to your [`init.fish`](https://github.com/oh-my-fish/oh-my-fish#dotfiles):
`set --export VIRTUAL_ENV_DISABLE_PROMPT 1`
* Indicate vi mode. (If you've set `fish_vi_mode` in your config and don't like the ugly left prompt indicator you can solve this by replacing it with `set -g fish_key_bindings fish_vi_key_bindings` and then removing the `if set -q __fish_vi_mode` check at the bottom of the `fish_right_prompt.fish`)

* Indicate vi mode. You will probably want to disable the default vi mode prompt. Add to your [`init.fish`](https://github.com/oh-my-fish/oh-my-fish#dotfiles): `function fish_mode_prompt; end function`
Copy link
Member

Choose a reason for hiding this comment

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

The command should be function fish_mode_prompt; end. There's no end function 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ow yeah, good catch.

@derekstavis
Copy link
Member

derekstavis commented Aug 7, 2016

The best thing to do if you'd like the theme to automatically remove the left vi indicator is to define an empty fish_mode_prompt function in fish_mode_prompt.fish file.

Default unset state of `theme_host_hostname` will still mean auto-mode, but explicit `no` value will force to always show a hostname.
@olbrew
Copy link
Contributor Author

olbrew commented Aug 8, 2016

@derekstavis That's indeed even better. I've added it to the right prompt. That seems better than creating a new file just for the purpose of defining an empty function. I've also squashed the commits to have a cleaner history.

Rewrites the right prompt it with native fish-shell functions, fixes the coloring
issue seen before and adds a new check for the vi mode used from fishv v2.3
onward so it is only shown when vi mode is enabled.
@sn0cr
Copy link
Collaborator

sn0cr commented Sep 22, 2016

Sorry that I'm replying that late.
Are you ready to merge?

@olbrew
Copy link
Contributor Author

olbrew commented Sep 22, 2016

Yep, should be ready to go. I use it myself and it works against the latest version 👍.

@sn0cr sn0cr merged commit 9ef8418 into oh-my-fish:master Sep 22, 2016
@sn0cr
Copy link
Collaborator

sn0cr commented Sep 22, 2016

Thank you very much @olbrew for your contribution and @derekstavis for the help.

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

4 participants