Powerlevel9k / powerlevel9k

Powerlevel9k was a tool for building a beautiful and highly functional CLI, customized for you. P9k had a substantial impact on CLI UX, and its legacy is now continued by P10k.
https://github.com/romkatv/powerlevel10k
MIT License
13.47k stars 947 forks source link

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

Closed wrboyce closed 5 years ago

wrboyce commented 5 years ago

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 commented 5 years ago

ping @Syphdias @bhilburn is this repo still maintained?

wrboyce commented 5 years ago

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

dritter commented 5 years ago

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 daa7255e85e939293bca28e93c8a058425368243 ) 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?