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

Bug report: wd show command does not work for directories under home #105

Closed
kelbyers opened this issue Feb 17, 2022 · 16 comments
Closed

Bug report: wd show command does not work for directories under home #105

kelbyers opened this issue Feb 17, 2022 · 16 comments
Labels

Comments

@kelbyers
Copy link
Contributor

Describe the bug
wd show is not correctly showing warp points that are located within my home directory structure. Warp points that just go to $HOME do show up with the show command.

To Reproduce
Steps to reproduce the behavior:

  1. create and cd to a temporary unique directory in your home directory: cd $(mktemp -d ~/tmp.XXXXXXXX)
  2. add a warp point to the directory: wd add mytmp1
  3. make sure the warp point was added: wd list
  4. check for defined warp points to the current directory: wd show

Expected behavior
A response that one warp point is defined for the current directory:

 * 1 warp point(s) to current directory: mytmp1

Screenshots

➜ cd $(mktemp -d ~/tmp.XXXXXXXX)
➜ wd add mytmp1
 * Warp point added
➜ wd list
 * All warp points:
ci-cd-scripts  ->  ~/git/ose-automation-stores/ci-cd-scripts
       mytmp1  ->  ~/tmp.7vczbdgF
➜ wd show
 * No warp point to ~/tmp.7vczbdgF

Desktop (please complete the following information):

  • zsh version: zsh 5.8 (x86_64-apple-darwin19.6.0)
  • wd version: wd version 0.5.0

Additional context
I do see that there is a version of wd tagged as 0.5.1. The version that I have is installed using zgenom load mfaerevaag/wd, and it displays its version as 0.5.0.

@kelbyers kelbyers added the Bug label Feb 17, 2022
@altschuler
Copy link
Collaborator

Strange, I can't reproduce with exactly your steps. Does wd show work for other warp points?

@kelbyers
Copy link
Contributor Author

kelbyers commented Feb 18, 2022

wd show works for anything that is not a subdirectory of my home directory. Also, it does not matter whether I specify a full path or not when adding the warp point (although I think I played around with specifying the second parameter to wd add and it might be ignoring everything after the first parameter):

➜ cd /usr/local

➜ wd add ul
 * Warp point added

➜ wd show
 * 1 warp point(s) to current directory: ul

➜ wd list
 * All warp points:
ci-cd-scripts  ->  ~/git/ose-automation-stores/ci-cd-scripts
   st-promote  ->  ~/git/ose-automation-stores/ci-cd-scripts
           ul  ->  /usr/local
       wrench  ->  ~/git/ose-automation-puppet/service-now-wrench

➜ cd

➜ cd Applications

➜ pwd
/Users/jxb2016/Applications

➜ wd add apps /Users/jxb2016/Applications
 * Warp point added

➜ wd show
 * No warp point to ~/Applications

➜ wd list
 * All warp points:
         apps  ->  ~/Applications
ci-cd-scripts  ->  ~/git/ose-automation-stores/ci-cd-scripts
   st-promote  ->  ~/git/ose-automation-stores/ci-cd-scripts
           ul  ->  /usr/local
       wrench  ->  ~/git/ose-automation-puppet/service-now-wrench

@kelbyers
Copy link
Contributor Author

Hmm. wd show <point> does work:

➜ wd show apps
 * Warp point: apps -> ~/Applications

@altschuler
Copy link
Collaborator

Might be a macOS specific issue, I don't have one to test on though. Could you try to run the test suite? Just clone this repo, cd into to the test directory and run ./tests.sh.

@kelbyers
Copy link
Contributor Author

kelbyers commented Mar 4, 2022

Sorry about taking so long. I have run the tests:

➜ /bin/zsh ./tests.sh
test_empty_config
ASSERT:should initially be an empty config expected:<0> but was:<       0>
test_simple_add_remove
ASSERT:should have 1 wps expected:<1> but was:<       1>
ASSERT:wps should be empty expected:<0> but was:<       0>
test_default_add_remove
ASSERT:should have 1 wps expected:<1> but was:<       1>
ASSERT:wps should be empty expected:<0> but was:<       0>
test_no_duplicates
test_default_no_duplicates
test_default_multiple_directories
test_valid_identifiers
test_removal
test_list
ASSERT:should only be one warp point expected:<       2> but was:<2>
ASSERT:should be more than one warp point expected:<       3> but was:<3>
test_show
test_quiet
test_clean
test_ls
test_path
test_config
ASSERT:expected:<1> but was:<       1>

Ran 15 tests.

FAILED (failures=8)

@kelbyers
Copy link
Contributor Author

kelbyers commented Mar 4, 2022

Looks to me like the difference is in the output of the wc command. The version that comes with MacOS seems to indent the output.

➜ grep wd tests.sh | wc -l
      72

Here is the output from gnu's wc:

wd/test at C02W11Y6HTD6MBP on  master
➜ grep wd tests.sh | gwc -l
72

@kelbyers
Copy link
Contributor Author

kelbyers commented Mar 4, 2022

One thing that I notice looking at the code for wd_show. When figuring out the current directory to look for in the list of warp points, it runs: PWD="${PWD/$HOME/~}". My understanding is that the intent of that is turn something like /Users/kelbyers/git into ~/git. That doesn't seem to work, at least on my MacOS:

➜ echo $HOME
/Users/kelbyers

➜ echo $PWD
/Users/kelbyers/git/mfaerevaag/wd/test

➜ echo ${PWD/$HOME/~}
/Users/kelbyers/git/mfaerevaag/wd/test

The substitution does work, in general, but I think the / in $HOME are effecting it:

➜ echo ${PWD/kelbyers/HOME}
/Users/HOME/git/mfaerevaag/wd/test

@kelbyers
Copy link
Contributor Author

kelbyers commented Mar 4, 2022

Well, ignore my comment above. I looks like the substitution does work when I quote it:

➜ echo "${PWD/$HOME/~}"
~/git/mfaerevaag/wd/test

@kelbyers
Copy link
Contributor Author

kelbyers commented Mar 4, 2022

Running with debug output (set -x ; wd show):

➜ pwd
/Users/kelbyers/git
➜ wd list | grep ' git'
          git  ->  ~/git
+/Users/jxb2016/git/mfaerevaag/wd/wd.sh:460> wd_show ''
+wd_show:2> local name_arg=''
+wd_show:4> [[ -n '' ]]
+wd_show:14> local wd_matches
+wd_show:15> wd_matches=( )
+wd_show:17> PWD='~/git'
+wd_show:18> [[ '' == \~/git ]]
+wd_show:30> echo '~/git'
+wd_show:30> sed 's:/Users/kelbyers:~:'
+wd_show:30> wd_print_msg '\033[93m' 'No warp point to ~/git'
+wd_print_msg:2> [[ -z '' ]]
+wd_print_msg:4> local color='\033[93m'
+wd_print_msg:5> local msg='No warp point to ~/git'
+wd_print_msg:7> [[ '\033[93m' ==  || 'No warp point to ~/git' ==  ]]
+wd_print_msg:11> print ' \033[93m*\033[m No warp point to ~/git'
 * No warp point to ~/git

isen0011 added a commit to isen0011/wd that referenced this issue Mar 13, 2022
As noticed in mfaerevaag#105, MacOS `wc -l` produces extra whitespace padding at
the front of the word count.  This makes it so that running the tests on
MacOS produces false negatives.

The fix here is to add a new function that calls `wc -l` then uses `sed`
to strip out the extra whitespace.  This is only used in the tests.
After running this on a MacOS machine, all tests pass as expected.
alpha-tango-kilo pushed a commit that referenced this issue Mar 13, 2022
As noticed in #105, MacOS `wc -l` produces extra whitespace padding at
the front of the word count.  This makes it so that running the tests on
MacOS produces false negatives.

The fix here is to add a new function that calls `wc -l` then uses `sed`
to strip out the extra whitespace.  This is only used in the tests.
After running this on a MacOS machine, all tests pass as expected.
@alpha-tango-kilo
Copy link
Collaborator

Looks like the issue may be being caused by how the warp point is formatted when it's saved - it's not matching anything when it's being looked up here.

From your WD_CONFIG (which is ~/.warprc IIRC, unless you've changed it), how is the warp point saved? Is the path /Users/kelbyers/git, or ~/git? I'm expecting the former, else I have no clue why the lookup fails

@alpha-tango-kilo
Copy link
Collaborator

alpha-tango-kilo commented Mar 13, 2022

Alternatively, adding an echo "$points" after points is loaded may show the same information

@kelbyers
Copy link
Contributor Author

the saved warp points are like this:

➜ cat ~/.warprc
ci-cd-scripts:~/git/ose-automation-stores/ci-cd-scripts
git:~/git
puppet-ci:~/git/ose-automation-puppet/puppet-ci
st-promote:~/git/ose-automation-stores/ci-cd-scripts
thdlib:~/git/ose-automation-puppet/thdlib
trinity1:~/git/ose-automation-puppet/puppet-ci
wrench:~/git/ose-automation-puppet/service-now-wrench

@tomterl
Copy link
Contributor

tomterl commented Oct 6, 2022

I have the same symptoms and it looks like the zsh option extended_glob is causing this at my end:

~/projects/zshorg master[8fc846b] ✔ $                                                                                                               [12:09]
 ❯ wd show
 * No warp point to ~/projects/zshorg

 ~/projects/zshorg master[8fc846b] ✔ $                                                                                                               [12:09]
 ❯
 ❯ setopt noextendedglob

 ~/projects/zshorg master[8fc846b] ✔ $                                                                                                               [12:10]
 ❯ wd show
 * 1 warp point(s) to current directory: zo

 ~/projects/zshorg master[8fc846b] ✔ $                                                                                                               [12:10]

Before I turned to my zsh setup and deactivated active options one at a time (kidding, I was going to, but extended_glob was the first candidate), I introduced debug prints all over the place, and that was strange:

* Comparing '~/projects/zshorg' -> '~/projects/zshorg'

The strings look the same, but aren't from zsh's point of view.

I'm using ubuntu 20.04, zsh 5.8.

tomterl added a commit to tomterl/wd that referenced this issue Oct 6, 2022
This fixes the behaviour described in mfaerevaag#105 for my setup.
tomterl added a commit to tomterl/wd that referenced this issue Oct 6, 2022
This fixes the behaviour described in mfaerevaag#105 for my setup.
@alpha-tango-kilo
Copy link
Collaborator

Comparing '~/projects/zshorg' -> '~/projects/zshorg'

This is why I never found the issue, because the things were literally equal (to my puny human eyes), thank you for finding this!

@tomterl
Copy link
Contributor

tomterl commented Oct 6, 2022

@kelbyers do you have extend_glob set in your setup, or is this possibly something else on macOS?

alpha-tango-kilo pushed a commit that referenced this issue Oct 6, 2022
This fixes the behaviour described in #105 for my setup.
@alpha-tango-kilo
Copy link
Collaborator

alpha-tango-kilo commented Oct 6, 2022

Closing as fixed until we hear otherwise, I'll cut a v0.5.2 release shortly. Thank you very much tomterl!

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

No branches or pull requests

4 participants