BUGFIX: Repair broken "t" and "T" mappings #759
Conversation
This method needed some love. The internals were simplified and reformatted, and the comment was edited for additional readability.
This bug is subtle! Opening a directory node in a new tab (with the "t" or "T" mappings) would previously fail and require a refresh because it called the directory node's "activate()" method. In reviewing that method (i.e., "activate()"), I discovered that the directory node's NERDTree is rendered before the method returns, which overwrites the content of the tree in the new tab or window. To clarify, when "t" or "T" is used on a directory node, a new directory node and tree must be created to be rendered in a new tab. So, calling "self.getNerdtree().render()" at the bottom of "activate()" will render the NERDTree instance from which "t" or "T" was invoked, not the new NERDTree that is being displayed. This overwrites the new NERDTree text, and, thus, a refresh is required. Since a call to "render()" is almost always necessary at the bottom of "activate()" to keep everything in sync for other mappings, we avoid this problem entirely by using the "open()" method directly (works for files and directories) in the callbacks. Fixes preservim#549.
The "t" and "T" mappings didn't work on bookmarks. This commit fixes this problem by making the callbacks more general. Fixes preservim#565.
This warning makes developers aware of the possibility of overwriting the NERDTree text in a new window when activate is used.
|
Thanks, @PhilRunninger!!! As one last adjustment, do you think it would be a good idea to open tabs at the end of all open tabs, like with Either way, the tab can be moved after the fact to the desired location. It's a minor change, but I would like to see it added. Since the mappings were already broken, no one depended on them anyway... |
|
I have no opinion on this one, since I don't use tabs. Opening tabs at the end is consistent with other IDEs, so it's probably OK. Since is it new behavior, I would suggest putting a mention in the FAQ Wiki, explaining the change and the command to move tabs after they're created. |
|
Got it. I'll add it to the |
|
The I'll merge now and update the Wiki in a few minutes. |
| "TODO: This is kept for compatability only since many things use | ||
| "b:NERDTreeRoot instead of the new NERDTree.root | ||
| "Remove this one day | ||
| let b:NERDTreeRoot = b:NERDTree.root |
There was a problem hiding this comment.
Hi @lifecrisis
This change is causing Projectionist plugin to raise errors at startup.
tpope/vim-projectionist#89
I just want to highlight that but this is indeed something to be fixed in Projectionist
There was a problem hiding this comment.
Yup, we'll add that back. It's been a long time, so others should also be encouraged to update their code.
The title says it all. Pay particular attention to the commit message for ae1c95b. That commit really fixes the bug for directories, and the following commit fixes the same issue for bookmarks.
It's funny that
open()has a different signature for bookmark nodes than for file and directory nodes. This made the change more difficult to make general, but still doable.