jorgebucaran/hydro

Prompt doesn't handle ' in path

giorgiga opened this issue · 11 comments

Just noticed this, thanks to the "Sid Meier's Civilization VI" folder:

~ ❱ mkdir "a'b"
~ ❱ cd a\'b/
~/ab\x1b[0m \x1b[0m\x1b[0m❱\x1b[0m 

Nice catch! We need to stop using string unescape in fish_prompt.

Any luck coming up with a workaround for this @giorgiga? 🤔

I hadn't actually looked at the code 😁
I'll do it and submit a PR if I can figure things out (or comment here with my findings if I can't)

I submitted a PR but then delete it as I realised I didn't 100% understand the code of the _hydro_pwd function (copypasted here for ease of reference):

function _hydro_pwd --on-variable PWD
    set --query fish_prompt_pwd_dir_length || set --local fish_prompt_pwd_dir_length 1
    if test "$fish_prompt_pwd_dir_length" -le 0 || test "$hydro_multiline" = true
        set --global _hydro_pwd (
            string replace --ignore-case -- ~ \~ $PWD |
            string replace --regex -- '([^/]+)$' "\x1b[1m\$1\x1b[22m" |
            string replace --regex --all -- '(?!^/$)/' "\x1b[2m/\x1b[22m"
        )
    else
        set --local root (command git rev-parse --show-toplevel 2>/dev/null |
            string replace --all --regex -- "^.*/" "")
        set --global _hydro_pwd (
            string replace --ignore-case -- ~ \~ $PWD |
            string replace -- "/$root/" /:/ |
            string replace --regex --all -- "(\.?[^/]{"$fish_prompt_pwd_dir_length"})[^/]*/" \$1/ |
            string replace -- : "$root" |
            string replace --regex -- '([^/]+)$' "\x1b[1m\$1\x1b[22m" |
            string replace --regex --all -- '(?!^/$)/' "\x1b[2m/\x1b[22m"
        )
    end
    test "$root" != "$_hydro_git_root" &&
        set --global _hydro_git_root $root && set $_hydro_git
end

The bits I don't understand are:

  1. What is the function of that string replace --all --regex -- "^.*/" "" when getting the git root directory? (doesn't git rev-parse --show-toplevel always print the absolute path?)
  2. Doesn't the last statement always get skipped since root is a variable local to the previous else block? (is there a root global variable too?)
  3. What do the last 2 substitions (string replace --regex -- '([^/]+)$' "\x1b[1m\$1\x1b[22m" | string replace --regex --all -- '(?!^/$)/' "\x1b[2m/\x1b[22m") do?

Ok, so... based on these assumed answers:

  1. It serves no purpose: delete it
  2. it serves no purpose: delete it
  3. the first highlights the "main" directory (the current one or the one of the git repo we are in), the second one dims the path separators

I've rewritten _hydro_pwd and fish_prompt: please take a look at giorgiga@db82440 and let me know what you think (personally, I'd like to make the dimming of path separators configurable too, but I wanted to hear what you say first)

Read the code again after a pause.

The string replace --all --regex -- "^.*/" "" bit extracts the last path element from the git root directory path (ie. the same function as filename) and then that specific element is searched for and replaced with a colon further down (string replace -- "/$root/" /:/)... seems like this would cause problems if the git root directory name is not unique in the path (eg. /home/user/work/project/work). I think the sketched implementation in my copy of the repo, where PWD is explicitly split into three pieces, could be an improvement over this.

The last line (test "$root" != "$_hydro_git_root" && set --global _hydro_git_root $root && set $_hydro_git) is still beyond my comprehension... it looks like $root is not defined there and so the code just resets _hydro_git_root and the variable whose name is contained in _hydro_git whatever _hydro_git_root is not empty, but (unless I am mistaken) that is the only place _hydro_git_root ever receives a value and so... it's always a noop? IDK it's too late at night to say 😁

I ended up writing something that is probably overkill 😄

Screenshot_20220218_172829

$PWD is split in 4 sections (say, before git, git dir, path to current dir, current dir) and one can specify how many path elements to select for sections "git dir" and "current dir" (2 purple and 1 yellow in the screenshot above) and for each section also:

  • the max length of directory names to show unabbrebviated
  • how many chars to pick from the left and right when abbreviating
  • color of directory names
  • color of path separators

The code is longer than the current _hydro_pwd, but I'd say it's not outrageously more complex... is it something you might accept as a contribution or is too much overkill? (asking because it's fine as it is for my personal use, but I'd have to clean it up to send it in a PR - not a big deal but I'd rather not do it if you tell me you are not interested in this functionality for hydro)

We just have to remove the one usage of string unescape and replace the embedded ANSI escape codes with Fish native colors via set_color.

Removing string unescape seems to solve the problem, but... hadn't it been added for some reason in the first place?

Anyway if there's no need for additional investigation and no interest in extended functionality, I guess waiting for my PR will only slow you down... there's still stuff I don't understand about the code (eg. hydro_color_pwd vs _hydro_color_pwd) and it would take a while.

Was added to unescape hardcoded ANSI escape codes. I was trying to save space, but as we can see now it was pointless.

Fixed in 83c6f26.