Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

[Bugfix] dir: Fix package name path truncation inside home dir #1158

Merged
merged 1 commit into from
Mar 1, 2019

Conversation

wrboyce
Copy link

@wrboyce wrboyce commented Feb 6, 2019

Description

Without this, current_path would be ~/foo/bar whereas package_path would be /home/user/foo/bar which breaks the substring replacement when calculating the subdirectory relative to the package (making the truncation far too greedy).

Questions

I presume there is a reason for the complexity in this:

# Replace the shortest possible match of the marked folder from
# the current path. Remove the amount of characters up to the
# folder marker from the left. Count only the visible characters
# in the path (this is done by the "zero" pattern; see
# http://stackoverflow.com/a/40855342/5586433).
local zero='%([BSUbfksu]|([FB]|){*})'
# Then, find the length of the package_path string, and save the
# subdirectory path as a substring of the current directory's path from 0
# to the length of the package path's string
subdirectory_path=$(__p9k_truncate_path "${current_path:${#${(S%%)package_path//$~zero/}}}" $P9K_DIR_SHORTEN_LENGTH $P9K_DIR_SHORTEN_DELIMITER)

And that something simpler would not work? Something like this

subdirectory_path=$(__p9k_truncate_path "${current_path/${package_path}/}" $P9K_DIR_SHORTEN_LENGTH $P9K_DIR_SHORTEN_DELIMITER)

@wrboyce wrboyce changed the title [Bugfix] fix package name path truncation inside home dir [Bugfix] dir: Fix package name path truncation inside home dir Feb 7, 2019
@wrboyce
Copy link
Author

wrboyce commented Feb 28, 2019

ping @Syphdias @bhilburn is this repo still maintained?

@wrboyce
Copy link
Author

wrboyce commented Feb 28, 2019

ping @dritter too as you seem to do all the merging here.

@dritter
Copy link
Member

dritter commented Mar 1, 2019

Hi @wrboyce ,

Yes, this repo is still maintained. Sorry for the delay.

About the zero pattern: I remember writing this comment, but unfortunately not why it was done in the first place. A quick annotate didn't bring light into the dark either.
Anyways, the code originated from #229 (Commit daa7255 ) and I think it can be simplified as you suggested.

About how we proceed with this PR: We are close to releasing a new Version and I think this Bug affects master as well. Could you port your fix over?

@dritter dritter merged commit 39d631c into Powerlevel9k:next Mar 1, 2019
dritter added a commit to dritter/powerlevel9k that referenced this pull request Mar 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants