preservim / nerdtree

A tree explorer plugin for vim.
Do What The F*ck You Want To Public License
19.66k stars 1.45k forks source link

Add g:NERDTreeFindResolveSymlinks for NERDTreeFind #1412

Open EvgeniyBlinov opened 8 months ago

EvgeniyBlinov commented 8 months ago

Description of Changes

Add g:NERDTreeFindResolveSymlinks for NERDTreeFind

" DEFAULT value | open NERDTreeFind in symlink resolved dir let g:NERDTreeFindResolveSymlinks = 1

" open NERDTreeFind in file current dir let g:NERDTreeFindResolveSymlinks = 0


New Version Info

rzvxa commented 8 months ago

Hello,

First off thank you for your contribution. It seems like a good idea but I'm not sure if using a flag for it is the best implementation, I personally think having 2 commands like NERDTreeFind and NERDTreeFindSymlink is a better way of doing this but we can discuss if you have a good use case for this approach.

There are also #1404 and issues mentioned there, which are somewhat related to this, Take a look at them if have the time, it would be great if we could work toward having better control over how NerdTree is treating the symlinks in general. For example, having the Path.Resolve method handle how all symlinks are treated seems like a good solution to me however we should make sure there are no weird assumptions throughout the code that could break it.

EvgeniyBlinov commented 8 months ago

Please, add any solution. Because current NERDTreeFind blocking my workflow.

rzvxa commented 8 months ago

It is definitely on the roadmap, I usually don't go overboard with symlinks but I know a lot of people and projects that do. So keep an eye out for the next few updates.

Until then feel free to use the patch you've submitted or get on board to help with adding an option for symlink resolution in general, Either way, I'm going to make sure that you get credited for this PR no matter what(even if the proposed option doesn't get in).

Have a great day!

EvgeniyBlinov commented 7 months ago

Please, add any solution. Because current NERDTreeFind blocking my workflow.

@rzvxa up Please, add any solution.

rzvxa commented 7 months ago

@EvgeniyBlinov Sorry for it getting a bit delayed, I just want to let you know I haven't forgotten this issue; I'm currently just a bit swamped with a deadline.

I'll try to get it into upstream by the next week end, I might need a few more days I promise it won't be much longer than that.

Please excuse me for this inconvenience.

rzvxa commented 7 months ago

Just to be clear, I didn't merge it yet, since we have to make sure it wouldn't break anything. I'm not too keen on breaking anyone's workflow since NERDTree has been in a stable state for a long long time and people tend to forget about it.

I want you to know that I truly appreciate what you've done here, I hope you understand the reasoning behind my decision.

rzvxa commented 6 months ago

@EvgeniyBlinov I'm extremely sorry for the delay, I'll start working on it today and ping you when there is a working PR so you can test and review the changes.

rzvxa commented 6 months ago

@EvgeniyBlinov This doesn't work, Vim always resolves the path on expansion, Are you sure your changes are creating the desired effect? I can't reproduce what you've described here.

EvgeniyBlinov commented 6 months ago

@EvgeniyBlinov This doesn't work, Vim always resolves the path on expansion, Are you sure your changes are creating the desired effect? I can't reproduce what you've described here. Test case:

mkdir -p ~/TMP/nerdtreesltestdir/
cd ~/TMP/nerdtreesltestdir/
ln -s ~/.bashrc ./
vim ./.bashrc

NerdTree open in HOME directory. But the expected behavior is opening in the current directory ~/TMP/nerdtreesltestdir/ . On commit https://github.com/preservim/nerdtree/commit/f3a4d8eaa8ac10305e3d53851c976756ea9dc8e8

rzvxa commented 6 months ago

@EvgeniyBlinov This doesn't work, Vim always resolves the path on expansion, Are you sure your changes are creating the desired effect? I can't reproduce what you've described here. Test case:

mkdir -p ~/TMP/nerdtreesltestdir/
cd ~/TMP/nerdtreesltestdir/
ln -s ~/.bashrc ./
vim ./.bashrc

NerdTree open in HOME directory. But the expected behavior is opening in the current directory ~/TMP/nerdtreesltestdir/ . On commit f3a4d8e

Have you had any chance of fixing it by not resolving the path? I've tried your proposed change in this PR with your exact example, But it doesn't solve the issue.

The problem is that Vim's behavior for symlinks is to resolve the path, So in many places when you pass in a symlink it tries to expand it to the real path internally, and one way or another it would get resolved somewhere(it can happen in many different operations even a simple thing like 1 ctrl-g would give the resolved path).

To truly solve this issue we have to stop using internal functions like expand which does the resolution for us, There might be some other areas that need to be addressed. Overall it is more complicated than what I originally thought, Unless there is something that I'm missing.

Does the proposed change in this PR solve your issue? because I couldn't get the desired effect you've mentioned. If it does, can you share a short video of it with me? What is your Vim version? There might be something different between our setups.

EvgeniyBlinov commented 6 months ago

@EvgeniyBlinov This doesn't work, Vim always resolves the path on expansion, Are you sure your changes are creating the desired effect? I can't reproduce what you've described here. Test case:

mkdir -p ~/TMP/nerdtreesltestdir/
cd ~/TMP/nerdtreesltestdir/
ln -s ~/.bashrc ./
vim ./.bashrc

NerdTree open in HOME directory. But the expected behavior is opening in the current directory ~/TMP/nerdtreesltestdir/ . On commit f3a4d8e

Have you had any chance of fixing it by not resolving the path? I've tried your proposed change in this PR with your exact example, But it doesn't solve the issue.

The problem is that Vim's behavior for symlinks is to resolve the path, So in many places when you pass in a symlink it tries to expand it to the real path internally, and one way or another it would get resolved somewhere(it can happen in many different operations even a simple thing like 1 ctrl-g would give the resolved path).

To truly solve this issue we have to stop using internal functions like expand which does the resolution for us, There might be some other areas that need to be addressed. Overall it is more complicated than what I originally thought, Unless there is something that I'm missing.

Does the proposed change in this PR solve your issue? because I couldn't get the desired effect you've mentioned. If it does, can you share a short video of it with me? What is your Vim version? There might be something different between our setups.

Dockerfile

FROM ubuntu:24.04

SHELL ["/bin/bash", "-c"]

ARG DEBIAN_FRONTEND=noninteractive
ENV DEBIAN_FRONTEND=noninteractive

RUN apt-get update && apt-get install -y \
    vim \
    git \
    curl

USER ubuntu

WORKDIR /home/ubuntu

ENV HOME=/home/ubuntu

RUN mkdir -p /home/ubuntu/.vim/pack/vendor/start && \
    mkdir -p /home/ubuntu/.vim/pack/vendor/opt

RUN git clone https://github.com/preservim/nerdtree /home/ubuntu/.vim/pack/vendor/opt/nerdtree-bug  && \
    cd $_ && \
    git checkout f3a4d8e

RUN git clone https://github.com/EvgeniyBlinov/nerdtree /home/ubuntu/.vim/pack/vendor/opt/nerdtree-patched && \
    cd $_ && \
    git checkout findAndRevealPath_symlink_resolve

RUN mkdir -p ~/TMP/nerdtreesltestdir/ && \
    cd ~/TMP/nerdtreesltestdir/ && \
    ln -s ~/.bashrc ./

vim version

vim --version
VIM - Vi IMproved 9.1 (2024 Jan 02, compiled Mar 31 2024 00:15:53)
Included patches: 1-16
Modified by team+vim@tracker.debian.org
Compiled by team+vim@tracker.debian.org
Huge version without GUI.  Features included (+) or not (-):
+acl               +file_in_path      +mouse_urxvt       -tag_any_white
+arabic            +find_in_path      +mouse_xterm       -tcl
+autocmd           +float             +multi_byte        +termguicolors
+autochdir         +folding           +multi_lang        +terminal
-autoservername    -footer            -mzscheme          +terminfo
-balloon_eval      +fork()            +netbeans_intg     +termresponse
+balloon_eval_term +gettext           +num64             +textobjects
-browse            -hangul_input      +packages          +textprop
++builtin_terms    +iconv             +path_extra        +timers
+byte_offset       +insert_expand     -perl              +title
+channel           +ipv6              +persistent_undo   -toolbar
+cindent           +job               +popupwin          +user_commands
-clientserver      +jumplist          +postscript        +vartabs
-clipboard         +keymap            +printer           +vertsplit
+cmdline_compl     +lambda            +profile           +vim9script
+cmdline_hist      +langmap           -python            +viminfo
+cmdline_info      +libcall           +python3           +virtualedit
+comments          +linebreak         +quickfix          +visual
+conceal           +lispindent        +reltime           +visualextra
+cryptv            +listcmds          +rightleft         +vreplace
+cscope            +localmap          -ruby              +wildignore
+cursorbind        -lua               +scrollbind        +wildmenu
+cursorshape       +menu              +signs             +windows
+dialog_con        +mksession         +smartindent       +writebackup
+diff              +modify_fname      +sodium            -X11
+digraphs          +mouse             -sound             +xattr
-dnd               -mouseshape        +spell             -xfontset
-ebcdic            +mouse_dec         +startuptime       -xim
+emacs_tags        +mouse_gpm         +statusline        -xpm
+eval              -mouse_jsbterm     -sun_workshop      -xsmp
+ex_extra          +mouse_netterm     +syntax            -xterm_clipboard
+extra_search      +mouse_sgr         +tag_binary        -xterm_save
-farsi             -mouse_sysmouse    -tag_old_static
   system vimrc file: "/etc/vim/vimrc"
     user vimrc file: "$HOME/.vimrc"
 2nd user vimrc file: "~/.vim/vimrc"
      user exrc file: "$HOME/.exrc"
       defaults file: "$VIMRUNTIME/defaults.vim"
  fall-back for $VIM: "/usr/share/vim"
Compilation: gcc -c -I. -Iproto -DHAVE_CONFIG_H -Wdate-time -g -O2 -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -ffile-prefix-map=/build/vim-g8cgSd/vim-9.1.0016=. -flto=auto -ffat-lto-objects -fstack-protector-strong -fstack-clash-protection -Wformat -Werror=format-security -fcf-protection -fdebug-prefix-map=/build/vim-g8cgSd/vim-9.1.0016=/usr/src/vim-2:9.1.0016-1ubuntu7 -DSYS_VIMRC_FILE=\"/etc/vim/vimrc\" -DSYS_GVIMRC_FILE=\"/etc/vim/gvimrc\" -D_REENTRANT -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
Linking: gcc -Wl,-Bsymbolic-functions -flto=auto -ffat-lto-objects -Wl,-z,relro -Wl,-z,now -Wl,--as-needed -o vim -lm -ltinfo -lselinux -lsodium -lacl -lattr -lgpm -L/usr/lib/python3.12/config-3.12-x86_64-linux-gnu -lpython3.12 -ldl -lm

Commands

## build
docker build -t vim-test .

## run
docker run --rm --name vim-test vim-test tail -f /dev/null

## exec into container
docker exec -it vim-test bash

Commands in container

## bug reproducing
( cd /home/ubuntu/.vim/pack/vendor/start && ln -s ./../opt/nerdtree-bug ./nerdtree )
vim ~/TMP/nerdtreesltestdir/.bashrc +:NERDTreeFind

## clear package
rm /home/ubuntu/.vim/pack/vendor/start/nerdtree

## reproducing patched version
( cd /home/ubuntu/.vim/pack/vendor/start && ln -s ./../opt/nerdtree-patched ./nerdtree )
echo 'let g:NERDTreeFindResolveSymlinks = 0' > /home/ubuntu/.vim/vimrc
vim ~/TMP/nerdtreesltestdir/.bashrc +:NERDTreeFind
rzvxa commented 6 months ago

Awesome, It is one of the more elaborate reproduction steps I've ever seen here. Let me study it and I'll reach out to you.

EvgeniyBlinov commented 5 months ago

Awesome, It is one of the more elaborate reproduction steps I've ever seen here. Let me study it and I'll reach out to you.

Any ideas?

EvgeniyBlinov commented 2 months ago

Awesome, It is one of the more elaborate reproduction steps I've ever seen here. Let me study it and I'll reach out to you.

UP