-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
feat: Support git relative path on pure layout #108
feat: Support git relative path on pure layout #108
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for this contribution @CrystalJewell 🎉
I am always impressed when someone can just jump in the code and somewhat understand the codebase, and pull out a fix like this. I find zsh code myself to be pretty unreadable!
I added some minor comments. Thanks again!
@reobin I've pushed up those changes for you to take a look at. If there is anything else let me know 😄 You're right, zsh isn't the most fun to try to understand, thankfully you're naming helped a lot though. I'm happy I'm able to help out! |
typewritten.zsh
Outdated
} | ||
|
||
tw_redraw() { | ||
tw_home_relative_wd="$(tw_get_home_relative_wd)" | ||
tw_git_relative_wd="$(tw_get_git_relative_wd $tw_home_relative_wd $tw_prompt_data[tw_git_branch] $tw_prompt_data[tw_git_home])" | ||
tw_displayed_wd="$(tw_get_displayed_wd $tw_home_relative_wd $tw_prompt_data[tw_git_branch] $tw_prompt_data[tw_git_home])" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion(non-blocking): Since tw_home_relative_wd
is now only used inside of the tw_get_displayed_wd
function, let's move the variable inside and not pass it as a param anymore
typewritten.zsh
Outdated
local tw_home_relative_wd="$1" | ||
local tw_git_branch="$2" | ||
local tw_git_home="$3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@reobin quick question about these. Since we're no longer sending tw_home_relative_wd
as a param, do you feel strongly about no longer sending params to this function in general? as in changing this block to
local tw_home_relative_wd="$(tw_get_home_relative_wd)"
local tw_git_branch="$tw_prompt_data[tw_git_branch]"
local tw_git_home="$tw_prompt_data[tw_git_home]"
Since those aren't used in any other place in tw_redraw()
except as params for tw_get_displayed_wd()
it almost makes sense to me that they are just set there but I'm not as versed in zsh as you are so I wanted to make sure this made sense to you as well before I did that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right! Good catch
While we're at it, there are some other cleanups we can do in the same section.
The tw_get_home_relative_wd
function is not very useful. We can merge that into tw_get_displayed_wd
.
Here's my suggestion for a new tw_get_displayed_wd
:
tw_get_displayed_wd() {
local tw_git_branch=$tw_prompt_data[tw_git_branch]
local tw_git_home=$tw_prompt_data[tw_git_home]
local tw_home_relative_wd="%~"
local tw_git_relative_wd="$tw_git_home%c"
local tw_displayed_wd="$tw_git_relative_wd"
# The pure layout defaults to home relative working directory, but allows customization
if [[ "$TYPEWRITTEN_PROMPT_LAYOUT" = "pure" && "$TYPEWRITTEN_RELATIVE_PATH" = "" ]]; then
tw_displayed_wd=$tw_home_relative_wd
fi;
if [[ "$TYPEWRITTEN_RELATIVE_PATH" = "home" ]]; then
tw_displayed_wd=$tw_home_relative_wd
fi;
if [[ "$TYPEWRITTEN_RELATIVE_PATH" = "adaptive" ]]; then
if [[ "$tw_git_branch" = "" ]]; then
tw_displayed_wd=$tw_home_relative_wd
fi;
fi;
echo "%F{$tw_current_directory_color}$tw_displayed_wd"
}
Mainly what changes here is that we only make 1 call to the color that the current directory should have, and that's on the return statement (echo
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, changes pushed up in c087e32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks perfect @CrystalJewell ! Thank you for your contribution 🎉
@all-contributors please add @CrystalJewell for code and ideas |
I've put up a pull request to add @CrystalJewell! 🎉 |
closes #107
Updates
pure
layout to allow for customization of theTYPEWRITTEN_RELATIVE_PATH
.This layout uses the
TYPEWRITTEN_RELATIVE_PATH="home"
by default so a check has been added intw_get_displayed_wd()
.TYPEWRITTEN_RELATIVE_PATH
is not set and the user has selectedTYPEWRITTEN_PROMPT_LAYOUT="pure"
we default to "home".Cleanup around
tw_get_displayed_wd()
based on changes is also included.Before
After