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 option to customize theme #2

Closed
rafaelrinaldi opened this issue Oct 13, 2015 · 10 comments
Closed

Add option to customize theme #2

rafaelrinaldi opened this issue Oct 13, 2015 · 10 comments
Labels
🚀 enhancement performance, UX or maintainability 🙏 help wanted needs PR or help on decision
Milestone

Comments

@rafaelrinaldi
Copy link
Collaborator

Since we already use variables everywhere, it should be as simple as changing them from local to the global scope.

@rafaelrinaldi rafaelrinaldi added the 🚀 enhancement performance, UX or maintainability label Oct 13, 2015
@rafaelrinaldi
Copy link
Collaborator Author

@oranja @edouard-lopez what's the best way to achieve this?

@oranja
Copy link
Contributor

oranja commented Feb 12, 2016

For bare fish (not using any framework), I believe the most standard way is to rearrange the variable assignments to serve as defaults in case the variable is not yet set.
For example:

# Symbols
set -q symbol_prompt; or set -g symbol_prompt ""
set -q symbol_git_down_arrow; or set -g symbol_git_down_arrow ""
set -q symbol_git_up_arrow; or set -g symbol_git_up_arrow ""
set -q symbol_git_dirty; or set -g symbol_git_dirty "*"
set -q symbol_horizontal_bar; or set -g symbol_horizontal_bar ""

This way, any of these variables can be defined in config.fish without being overwritten by pure's fish_prompt.fish.

If it gets too tedious, you can define a helper function:

function set_default -S -a var value
    set -q $var; or set -g $var $value
end

Then the previous block becomes:

# Symbols
set_default symbol_prompt ""
set_default symbol_git_down_arrow ""
set_default symbol_git_up_arrow ""
set_default symbol_git_dirty "*"
set_default symbol_horizontal_bar ""

In the price of indirection you get a more compact block, and it also helps to avoid bugs from typos...

@rafaelrinaldi
Copy link
Collaborator Author

@oranja that looks pretty decent and straightforward. Will give it a try! Thanks man.

@ghost
Copy link

ghost commented Feb 13, 2016

My two cents.

Mostly agree with @oranja, but please always prefix any global variables with the name of your plugin/theme/prompt/snippet/etc so as to not conflict with other global variables of the same name.

I personally recommend against using and or or. Long one-liners are difficult to maintain and modify in the future. But that's just my opinion.

My alternative is this:

set -l symbol_prompt ❯

if set -q pure_symbol_prompt
    set symbol_prompt $pure_symbol_prompt
end

Some may look at that and think "why do I need to create an extra variable and do in 4 LOC what I can do in 1?". The answer, again, has to do with readability and maintainability. Refactoring the code I have proposed is very easy and it's harder to botch causing a bug.

It's like a fortress you are building to protect yourself from your future self.

@rafaelrinaldi
Copy link
Collaborator Author

Great, I will keep that in mind. Thanks.

Rafael Rinaldi
http://rinaldi.io

On Feb 13, 2016, at 15:27, Jorge Bucaran notifications@github.com wrote:

Agree with @oranja, but please always prefix any global variables with the name of your plugin/theme/prompt/snippet/etc so as to not conflict (hardly, but still) with other global variables of the same name.


Reply to this email directly or view it on GitHub.

@edouard-lopez
Copy link
Member

Thanks for your knowledge!

@oranja good proposal
@bucaran nice improvement! Love future-proof argument!
don't we need -g to set globally?

@oranja
Copy link
Contributor

oranja commented Feb 15, 2016

What @bucaran is suggesting is to have two variables for each customization options.
The first is local and is the one actually used in the theme code.
The second can have any scope that affects the prompt - universal, global, or local to an outer function (i.e. a test function).
To clarify, here's @bucaran's suggestion again, with annotations. (hoping I won't get sued for copyright infringement here).

set -l symbol_prompt ❯    # This is the local variable that will be used in the prompt's code.

if set -q pure_symbol_prompt    # Here we check for any previous definition of `pure_symbol_prompt`. Not necessarily global.
    set symbol_prompt $pure_symbol_prompt    # We override the local variable with the external value, if it's present.
end

@oranja
Copy link
Contributor

oranja commented Feb 15, 2016

In the same file fish also has several of these set -q var; or set var
one liners, rendering your choice argument weaker.
Having said that, both are fine, but personally, I think that if you have
many of these in a row, it's better to use the shorter form. It's an
equally acceptable fish idiom.

On Tue, Feb 16, 2016, 00:22 Jorge Bucaran notifications@github.com wrote:

One more thing that I don't know whether it supports my argument or not,
is that fish does this too, so it's a way to stay consistent across the
universe.

https://github.com/fish-shell/fish-shell/blob/master/share/functions/__fish_config_interactive.fish#L18

don't we need -g to set globally?

Depends. I was assuming this variable will only be used in the file where
it was declared.


Reply to this email directly or view it on GitHub
#2 (comment).

@Shougo
Copy link
Contributor

Shougo commented Dec 7, 2016

I think this issue should be closed by #30.

@rafaelrinaldi
Copy link
Collaborator Author

@Shougo That's true. Thanks!

@edouard-lopez edouard-lopez added 🙏 help wanted needs PR or help on decision and removed help wanted labels Dec 12, 2018
edouard-lopez added a commit that referenced this issue Jan 15, 2019
# This is the 1st commit message:

rename `$pure_symbol_git_arrow_down` as `$pure_color_git_unpulled_commits`

# This is the commit message #2:

fix pure_color_git_unpulled_commits to pure_symbol_git_unpulled_commits
edouard-lopez added a commit that referenced this issue Jan 15, 2019
# This is the 1st commit message:

rename `$pure_symbol_git_arrow_down` as `$pure_color_git_unpulled_commits`

# This is the commit message #2:

fix pure_color_git_unpulled_commits to pure_symbol_git_unpulled_commits
edouard-lopez added a commit that referenced this issue Feb 18, 2019
# This is the 1st commit message:

rename `$pure_symbol_git_arrow_down` as `$pure_color_git_unpulled_commits`

# This is the commit message #2:

fix pure_color_git_unpulled_commits to pure_symbol_git_unpulled_commits
edouard-lopez added a commit that referenced this issue Aug 21, 2023
# This is the 1st commit message:

rename `$pure_symbol_git_arrow_down` as `$pure_color_git_unpulled_commits`

# This is the commit message #2:

fix pure_color_git_unpulled_commits to pure_symbol_git_unpulled_commits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚀 enhancement performance, UX or maintainability 🙏 help wanted needs PR or help on decision
Projects
None yet
Development

No branches or pull requests

4 participants