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

__bobthefish_pwd should use $PWD only #219

Open
LucaFilipozzi opened this issue Oct 26, 2019 · 10 comments
Open

__bobthefish_pwd should use $PWD only #219

LucaFilipozzi opened this issue Oct 26, 2019 · 10 comments

Comments

@LucaFilipozzi
Copy link
Contributor

LucaFilipozzi commented Oct 26, 2019

On macOS, root's $HOME is '/var/root' and '/var' is a symlink to '/private/var'.

Since __bobthefish_pwd uses pwd -P, bobthefish's prompt does not display '~' when $PWD is '/var/root' since pwd -Pyields '/private/var/root'.

Two possible fixes:

  • use pwd -L instead of pwd -P
  • use $PWD only

Let me know if you want a pull request.

@bobthecow
Copy link
Member

That was a fix for several other issues. I'm not sure what the right answer is, but it's not just switching to $PWD:

5273c92

@LucaFilipozzi
Copy link
Contributor Author

If we need normalized $PWD, then we need normalized $HOME.

In __bobthefish_path_segment, instead of case "$HOME", we can use case (builtin realpath "$HOME") instead.

Let me know if you want a pull request.

@bobthecow
Copy link
Member

I think that makes sense. I’d appreciate a PR :)

@bobthecow
Copy link
Member

Or if you’re feeling ambitious we could try to map back and forth between normalized and non-normalized paths:

#181 (comment)

That seems like the best answer, and also a huge headache :)

@LucaFilipozzi
Copy link
Contributor Author

I was in the process of making pull request (#221) when I saw your reply.

I suggest we make the small fix, for now. I'll give the issue (#181) a read this week.

@LucaFilipozzi
Copy link
Contributor Author

fish's builtin realpath man page indicates that "in general, scripts should not invoke the builtin directly" and that "they should just use [GNU] realpath".

This probably means that we should remove 'builtin' from my fix in #221.

As for #181, at least for git, we could use realpath --no-symlinks $PWD/(realpath --relative-to=$PWD (git rev-parse --show-toplevel)) to determine the top-level directory in a symlink-preserving way.

@LucaFilipozzi
Copy link
Contributor Author

LucaFilipozzi commented Oct 28, 2019

Find below two potential ways in which we could determine the git project directory.

option 1: path manipulation using realpath

function __bobthefish_git_project_dir -S -a real_pwd -d 'Print the current git project base directory'
    [ "$theme_display_git" = 'no' ]
    and return

    set -l is_inside_work_tree (command git rev-parse --is-inside-work-tree 2>/dev/null)
    or return # we are not in a git repo

    [ "$is_inside_work_tree" = "false" ]
    and return # we are in a git repo but not in the tree; we are probably in .git

    echo (command realpath --no-symlinks $PWD/(command realpath --relative-to=$PWD (git rev-parse --show-toplevel)))
end

option 2: path manipulation using string functions

function __bobthefish_git_project_dir -S -a real_pwd -d 'Print the current git project base directory'
    [ "$theme_display_git" = 'no' ]
    and return

    set -l is_inside_work_tree (command git rev-parse --is-inside-work-tree 2>/dev/null)
    or return # we are not in a git repo

    [ "$is_inside_work_tree" = "false" ]
    and return # we are in a git repo but not in the tree; we are probably in .git

    set -l prefix (string trim -r -c / (command git rev-parse --show-prefix))
    set -l prjdir (string replace -r "/$prefix\$" "" $PWD) # can result in edge case
    [ -z "$prjdir" ]; and set prjdir "/" # handle edge case where prjdir is /
    echo $prjdir
end

Note: these currently use $PWD because I have not modified function __bobthefish_pwd yet.

@LucaFilipozzi
Copy link
Contributor Author

LucaFilipozzi commented Oct 28, 2019

Combined with set -l real_pwd $PWD rather than set -l real_pwd (__bobthefish_pwd),

(1) the following function seems to Work For Me® in git repos:

function __bobthefish_git_project_dir -S -a real_pwd -d 'Print the current git project base dir
ectory'
    [ "$theme_display_git" = 'yes' ]
    or return

    set -q theme_vcs_ignore_paths
    and [ (__bobthefish_ignore_vcs_dir $real_pwd) ]
    and return

    set -l is_inside_work_tree (command git rev-parse --is-inside-work-tree 2>/dev/null)
    or return # we are not in a git repo

    [ "$is_inside_work_tree" = 'false' ]
    and return # we are in a git repo but not in the tree; we are probably in .git

    set -l prefix (string trim -r -c / (command git rev-parse --show-prefix))
    set -l prjdir (string replace -r "/$prefix\$" '' $real_pwd)
    [ -z "$prjdir" ]; and set prjdir '/' # handle edge case where prjdir is /
    echo $prjdir
end

(2) the following function appears to Work For Me® in hg repos:

function __bobthefish_hg_project_dir -S -a real_pwd -d 'Print the current hg project base direc
tory'
    [ "$theme_display_hg" = 'yes' ]
    or return

    set -q theme_vcs_ignore_paths
    and [ (__bobthefish_ignore_vcs_dir $real_pwd) ]
    and return

    set -l prjdir (command hg root --cwd $real_pwd 2>/dev/null)
    or return # we are not in an hg repo

    set -l prefix (string replace $prjdir/ '' (pwd -P))
    set prjdir (string replace -r "/$prefix\$" '' $real_pwd)
    [ -z "$prjdir" ]; and set prjdir '/' # handle edge case where prjdir is /
    echo $prjdir
end

This means that we don't need function __bobthefish_pwd and that we don't need case (builtin realpath $HOME).

It may also mean that we can clean up the passing of real_pwd.

@LucaFilipozzi
Copy link
Contributor Author

I have generated pull request #222.

@LucaFilipozzi
Copy link
Contributor Author

LucaFilipozzi commented Oct 29, 2019

I have made two additional commits to pull request #222 and added some comments.

Re "if you're feeling ambitious" ... challenge accepted 😄but I think I'm done for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants