Refactor sort comparison functions, removing redundancy (#1166)

* Add a function to compare path objects.

* Remove redundant node comparison function, and rename the ones left.

* Remove the compareTo function in the Path object.

Use nerdtree#compareNodePaths(p1,p2) instead. There was no need for two
comparison functions that do the same thing. They were a little
different in their details, but that shouldn't be the case. Having only
one such function makes better sense and is easier to maintain.

* Update version number in change log.

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,5 +1,5 @@
 ### Description of Changes
-Closes #  <!-- Issue number this PR addresses. If none, remove this line. -->
+Closes #    <!-- Enter the issue number this PR addresses. If none, remove this line. -->
 
 
 ---
@@ -13,4 +13,10 @@ Closes #  <!-- Issue number this PR addresses. If none, remove this line. -->
 - [ ] Update [CHANGELOG.md](https://github.com/scrooloose/nerdtree/blob/master/CHANGELOG.md), following the established pattern.
 #### Collaborator's Instructions
 - [ ] Review [CHANGELOG.md](https://github.com/scrooloose/nerdtree/blob/master/CHANGELOG.md), suggesting a different version number if necessary.
-- [ ] After merge, tag the merge commit, e.g. `git tag -a 3.1.4 -m "v3.1.4" && git push origin --tags`
+- [ ] After merging, tag the commit using these (Mac-compatible) bash commands:
+    ```bash
+    git checkout master
+    git pull
+    sed -n "$(grep -n -m2 '####' CHANGELOG.md | cut -f1 -d: | sed 'N;s/\n/,/')p" CHANGELOG.md | sed '$d'
+    git tag -a $(read -p "Tag Name: " tag;echo $tag) -m"$(git show --quiet --pretty=%s)";git push origin --tags
+    ```

CHANGELOG.md

@@ -4,6 +4,16 @@
      version in an unordered list.  The format is:
         - **.PATCH**: Pull Request Title (PR Author) [PR Number](Link to PR)
 -->
+#### 6.9
+- **.8**: Refactor sort comparison functions, removing redundancy (PhilRunninger) [#1166](https://github.com/preservim/nerdtree/pull/1166)
+- **.7**: Fix argument of `exists()` function calls checking for autocommands. (PhilRunninger) [#1165](https://github.com/preservim/nerdtree/pull/1165)
+- **.6**: Don't use silent when raising User events (PhilRunninger) [#1164](https://github.com/preservim/nerdtree/pull/1164)
+- **.5**: Fix highlight for file node.  (pirey) [#1157](https://github.com/preservim/nerdtree/pull/1157)
+- **.4**: Make sure symbolic links' flags are highlighted correctly.  (PhilRunninger) [#1156](https://github.com/preservim/nerdtree/pull/1156)
+- **.3**: Fix new NERDTrees' width when previous one was in the only window. (PhilRunninger) [#1153](https://github.com/preservim/nerdtree/pull/1153)
+- **.2**: Fix the scope of several key mappings (lifecrisis, PhilRunninger) [#1151](https://github.com/preservim/nerdtree/pull/1151)
+- **.1**: Respect user's `&shellslash` setting in CopyNode and RemoveNode functions (PhilRunninger) [#1150](https://github.com/preservim/nerdtree/pull/1150)
+- **.0**: Enable opening bookmarks in split windows. (PhilRunninger) [#1144](https://github.com/preservim/nerdtree/pull/1144)
 #### 6.8
 - **.0**: Allow concealed characters to show another character. (PhilRunninger) [#1138](https://github.com/preservim/nerdtree/pull/1138)
 #### 6.7

autoload/nerdtree.vim

@@ -30,9 +30,11 @@ endfunction
 " SECTION: General Functions {{{1
 "============================================================
 
-"FUNCTION: nerdtree#slash() {{{2
+" FUNCTION: nerdtree#slash() {{{2
+" Return the path separator used by the underlying file system.  Special
+" consideration is taken for the use of the 'shellslash' option on Windows
+" systems.
 function! nerdtree#slash() abort
-
     if nerdtree#runningWindows()
         if exists('+shellslash') && &shellslash
             return '/'
@@ -108,15 +110,15 @@ function! nerdtree#completeBookmarks(A,L,P) abort
     return filter(g:NERDTreeBookmark.BookmarkNames(), 'v:val =~# "^' . a:A . '"')
 endfunction
 
-"FUNCTION: nerdtree#compareNodes(dir) {{{2
+"FUNCTION: nerdtree#compareNodes(n1, n2) {{{2
 function! nerdtree#compareNodes(n1, n2) abort
-    return a:n1.path.compareTo(a:n2.path)
+    return nerdtree#compareNodePaths(a:n1.path, a:n2.path)
 endfunction
 
-"FUNCTION: nerdtree#compareNodesBySortKey(n1, n2) {{{2
-function! nerdtree#compareNodesBySortKey(n1, n2) abort
-    let sortKey1 = a:n1.path.getSortKey()
-    let sortKey2 = a:n2.path.getSortKey()
+"FUNCTION: nerdtree#compareNodePaths(p1, p2) {{{2
+function! nerdtree#compareNodePaths(p1, p2) abort
+    let sortKey1 = a:p1.getSortKey()
+    let sortKey2 = a:p2.getSortKey()
     let i = 0
     while i < min([len(sortKey1), len(sortKey2)])
         " Compare chunks upto common length.

autoload/nerdtree/ui_glue.vim

@@ -25,12 +25,16 @@ function! nerdtree#ui_glue#createDefaultBindings() abort
     call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapPreview, 'scope': 'Bookmark', 'callback': s.'previewBookmark' })
     call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapActivateNode, 'scope': 'all', 'callback': s.'activateAll' })
 
-    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapOpenSplit, 'scope': 'Node', 'callback': s.'openHSplit' })
-    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapOpenVSplit, 'scope': 'Node', 'callback': s.'openVSplit' })
+    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapOpenSplit, 'scope': 'FileNode', 'callback': s.'openHSplit' })
+    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapOpenSplit, 'scope': 'Bookmark', 'callback': s.'openHSplitBookmark' })
+    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapOpenVSplit, 'scope': 'FileNode', 'callback': s.'openVSplit' })
+    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapOpenVSplit, 'scope': 'Bookmark', 'callback': s.'openVSplitBookmark' })
 
-    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapPreview, 'scope': 'Node', 'callback': s.'previewNodeCurrent' })
-    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapPreviewVSplit, 'scope': 'Node', 'callback': s.'previewNodeVSplit' })
-    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapPreviewSplit, 'scope': 'Node', 'callback': s.'previewNodeHSplit' })
+    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapPreview, 'scope': 'FileNode', 'callback': s.'previewNodeCurrent' })
+    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapPreviewSplit, 'scope': 'FileNode', 'callback': s.'previewNodeHSplit' })
+    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapPreviewSplit, 'scope': 'Bookmark', 'callback': s.'previewNodeHSplitBookmark' })
+    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapPreviewVSplit, 'scope': 'FileNode', 'callback': s.'previewNodeVSplit' })
+    call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapPreviewVSplit, 'scope': 'Bookmark', 'callback': s.'previewNodeVSplitBookmark' })
 
     call NERDTreeAddKeyMap({ 'key': g:NERDTreeMapOpenRecursively, 'scope': 'DirNode', 'callback': s.'openNodeRecursively' })
 
@@ -511,6 +515,28 @@ function! s:openVSplit(target) abort
     call a:target.activate({'where': 'v'})
 endfunction
 
+"FUNCTION: s:openHSplitBookmark(bookmark) {{{1
+"handle the user activating a bookmark
+function! s:openHSplitBookmark(bm) abort
+    call a:bm.activate(b:NERDTree, !a:bm.path.isDirectory ? {'where': 'h'} : {})
+endfunction
+
+"FUNCTION: s:openVSplitBookmark(bookmark) {{{1
+"handle the user activating a bookmark
+function! s:openVSplitBookmark(bm) abort
+    call a:bm.activate(b:NERDTree, !a:bm.path.isDirectory ? {'where': 'v'} : {})
+endfunction
+
+" FUNCTION: s:previewHSplitBookmark(bookmark) {{{1
+function! s:previewNodeHSplitBookmark(bookmark) abort
+    call a:bookmark.activate(b:NERDTree, !a:bookmark.path.isDirectory ? {'stay': 1, 'where': 'h', 'keepopen': 1} : {})
+endfunction
+
+" FUNCTION: s:previewVSplitBookmark(bookmark) {{{1
+function! s:previewNodeVSplitBookmark(bookmark) abort
+    call a:bookmark.activate(b:NERDTree, !a:bookmark.path.isDirectory ? {'stay': 1, 'where': 'v', 'keepopen': 1} : {})
+endfunction
+
 " FUNCTION: s:openExplorer(node) {{{1
 function! s:openExplorer(node) abort
     call a:node.openExplorer()
@@ -538,11 +564,7 @@ endfunction
 
 " FUNCTION: s:previewBookmark(bookmark) {{{1
 function! s:previewBookmark(bookmark) abort
-    if a:bookmark.path.isDirectory
-        execute 'NERDTreeFind '.a:bookmark.path.str()
-    else
-        call a:bookmark.activate(b:NERDTree, {'stay': 1, 'where': 'p', 'keepopen': 1})
-    endif
+    call a:bookmark.activate(b:NERDTree, !a:bookmark.path.isDirectory ? {'stay': 1, 'where': 'h', 'keepopen': 1} : {})
 endfunction
 
 "FUNCTION: s:previewNodeCurrent(node) {{{1

doc/NERDTree.txt

@@ -318,9 +318,8 @@ Applies to: files.
 If a file node or a bookmark that links to a file is selected, it is opened in
 the previous window, but the cursor does not move.
 
-If a bookmark that links to a directory is selected, that directory is found
-in the current NERDTree. If the directory couldn't be found, a new NERDTree is
-created.
+If a bookmark that links to a directory is selected then that directory
+becomes the new root.
 
 The default key combo for this mapping is "g" + NERDTreeMapActivateNode (see
 |NERDTree-o|).
@@ -350,7 +349,7 @@ The same as |NERDTree-t| except that the focus is kept in the current tab.
                                                                     *NERDTree-i*
 Default key: i
 Map setting: *NERDTreeMapOpenSplit*
-Applies to: files.
+Applies to: files, and bookmarks pointing to files.
 
 Opens the selected file in a new split window and puts the cursor in the new
 window.
@@ -359,7 +358,7 @@ window.
                                                                    *NERDTree-gi*
 Default key: gi
 Map setting: *NERDTreeMapPreviewSplit*
-Applies to: files.
+Applies to: files, and bookmarks pointing to files.
 
 The same as |NERDTree-i| except that the cursor is not moved.
 
@@ -370,7 +369,7 @@ The default key combo for this mapping is "g" + NERDTreeMapOpenSplit (see
                                                                     *NERDTree-s*
 Default key: s
 Map setting: *NERDTreeMapOpenVSplit*
-Applies to: files.
+Applies to: files, and bookmarks pointing to files.
 
 Opens the selected file in a new vertically split window and puts the cursor
 in the new window.
@@ -379,7 +378,7 @@ in the new window.
                                                                    *NERDTree-gs*
 Default key: gs
 Map setting: *NERDTreeMapPreviewVSplit*
-Applies to: files.
+Applies to: files, and bookmarks pointing to files.
 
 The same as |NERDTree-s| except that the cursor is not moved.
 

lib/nerdtree/creator.vim

@@ -28,7 +28,9 @@ endfunction
 
 " FUNCTION: s:Creator._broadcastInitEvent() {{{1
 function! s:Creator._broadcastInitEvent()
-    silent doautocmd User NERDTreeInit
+    if exists('#User#NERDTreeInit')
+        doautocmd User NERDTreeInit
+    endif
 endfunction
 
 " FUNCTION: s:Creator.BufNamePrefix() {{{1
@@ -247,7 +249,7 @@ function! s:Creator._pathForString(str)
 
         "hack to get an absolute path if a relative path is given
         if dir =~# '^\.'
-            let dir = getcwd() . g:NERDTreePath.Slash() . dir
+            let dir = getcwd() . nerdtree#slash() . dir
         endif
 
         "hack to prevent removing slash if dir is the root of the file system.

lib/nerdtree/nerdtree.vim

@@ -27,7 +27,9 @@ function! s:NERDTree.changeRoot(node)
     call self.render()
     call self.root.putCursorHere(0, 0)
 
-    silent doautocmd User NERDTreeNewRoot
+    if exists('#User#NERDTreeNewRoot')
+        doautocmd User NERDTreeNewRoot
+    endif
 endfunction
 
 "FUNCTION: s:NERDTree.Close() {{{1

lib/nerdtree/path.vim

@@ -25,10 +25,10 @@ function! s:Path.AbsolutePathFor(pathStr)
     if l:prependWorkingDir
         let l:result = getcwd()
 
-        if l:result[-1:] ==# s:Path.Slash()
+        if l:result[-1:] == nerdtree#slash()
             let l:result = l:result . a:pathStr
         else
-            let l:result = l:result . s:Path.Slash() . a:pathStr
+            let l:result = l:result . nerdtree#slash() . a:pathStr
         endif
     endif
 
@@ -99,50 +99,6 @@ function! s:Path.changeToDir()
     endtry
 endfunction
 
-" FUNCTION: Path.compareTo() {{{1
-"
-" Compares this Path to the given path and returns 0 if they are equal, -1 if
-" this Path is 'less than' the given path, or 1 if it is 'greater'.
-"
-" Args:
-" path: the path object to compare this to
-"
-" Return:
-" 1, -1 or 0
-function! s:Path.compareTo(path)
-    let thisPath = self.getLastPathComponent(1)
-    let thatPath = a:path.getLastPathComponent(1)
-
-    "if the paths are the same then clearly we return 0
-    if thisPath ==# thatPath
-        return 0
-    endif
-
-    let thisSS = self.getSortOrderIndex()
-    let thatSS = a:path.getSortOrderIndex()
-
-    "compare the sort sequences, if they are different then the return
-    "value is easy
-    if thisSS < thatSS
-        return -1
-    elseif thisSS > thatSS
-        return 1
-    else
-        if !g:NERDTreeSortHiddenFirst
-            let thisPath = substitute(thisPath, '^[._]', '', '')
-            let thatPath = substitute(thatPath, '^[._]', '', '')
-        endif
-        "if the sort sequences are the same then compare the paths
-        "alphabetically
-        let pathCompare = g:NERDTreeCaseSensitiveSort ? thisPath <# thatPath : thisPath <? thatPath
-        if pathCompare
-            return -1
-        else
-            return 1
-        endif
-    endif
-endfunction
-
 " FUNCTION: Path.Create(fullpath) {{{1
 "
 " Factory method.
@@ -614,23 +570,6 @@ function! s:Path.New(pathStr)
     return l:newPath
 endfunction
 
-" FUNCTION: Path.Slash() {{{1
-" Return the path separator used by the underlying file system.  Special
-" consideration is taken for the use of the 'shellslash' option on Windows
-" systems.
-function! s:Path.Slash()
-
-    if nerdtree#runningWindows()
-        if exists('+shellslash') && &shellslash
-            return '/'
-        endif
-
-        return '\'
-    endif
-
-    return '/'
-endfunction
-
 " FUNCTION: Path.Resolve() {{{1
 " Invoke the vim resolve() function and return the result
 " This is necessary because in some versions of vim resolve() removes trailing
@@ -815,7 +754,7 @@ function! s:Path._strForEdit()
 
     " On Windows, the drive letter may be removed by "fnamemodify()".  Add it
     " back, if necessary.
-    if nerdtree#runningWindows() && l:result[0] ==# s:Path.Slash()
+    if nerdtree#runningWindows() && l:result[0] == nerdtree#slash()
         let l:result = self.drive . l:result
     endif
 
@@ -830,14 +769,14 @@ endfunction
 
 " FUNCTION: Path._strForGlob() {{{1
 function! s:Path._strForGlob()
-    let lead = s:Path.Slash()
+    let lead = nerdtree#slash()
 
     "if we are running windows then slap a drive letter on the front
     if nerdtree#runningWindows()
         let lead = self.drive . '\'
     endif
 
-    let toReturn = lead . join(self.pathSegments, s:Path.Slash())
+    let toReturn = lead . join(self.pathSegments, nerdtree#slash())
 
     if !nerdtree#runningWindows()
         let toReturn = escape(toReturn, self._escChars())
@@ -849,7 +788,7 @@ endfunction
 " Return the absolute pathname associated with this Path object.  The pathname
 " returned is appropriate for the underlying file system.
 function! s:Path._str()
-    let l:separator = s:Path.Slash()
+    let l:separator = nerdtree#slash()
     let l:leader = l:separator
 
     if nerdtree#runningWindows()

lib/nerdtree/tree_dir_node.vim

@@ -236,7 +236,7 @@ function! s:TreeDirNode.getChildIndex(path)
     let z = self.getChildCount()
     while a < z
         let mid = (a+z)/2
-        let diff = a:path.compareTo(self.children[mid].path)
+        let diff = nerdtree#compareNodePaths(a:path, self.children[mid].path)
 
         if diff ==# -1
             let z = mid
@@ -278,8 +278,8 @@ function! s:TreeDirNode._glob(pattern, all)
     else
         let l:pathSpec = escape(fnamemodify(self.path.str({'format': 'Glob'}), ':.'), ',')
 
-        " On Windows, the drive letter may be removed by fnamemodify().
-        if nerdtree#runningWindows() && l:pathSpec[0] ==# g:NERDTreePath.Slash()
+        " On Windows, the drive letter may be removed by "fnamemodify()".
+        if nerdtree#runningWindows() && l:pathSpec[0] == nerdtree#slash()
             let l:pathSpec = self.path.drive . l:pathSpec
         endif
     endif
@@ -666,7 +666,7 @@ function! s:TreeDirNode.sortChildren()
     if count(g:NERDTreeSortOrder, '*') < 1
         call add(g:NERDTreeSortOrder, '*')
     endif
-    let CompareFunc = function('nerdtree#compareNodesBySortKey')
+    let CompareFunc = function('nerdtree#compareNodes')
     call sort(self.children, CompareFunc)
     let g:NERDTreeOldSortOrder = g:NERDTreeSortOrder
 endfunction

lib/nerdtree/ui.vim

@@ -67,6 +67,10 @@ function! s:UI._dumpHelp()
         let help .= '" '. g:NERDTreeMapPreview .": find dir in tree\n"
         let help .= '" '. g:NERDTreeMapOpenInTab.": open in new tab\n"
         let help .= '" '. g:NERDTreeMapOpenInTabSilent .": open in new tab silently\n"
+        let help .= '" '. g:NERDTreeMapOpenSplit .": open split\n"
+        let help .= '" '. g:NERDTreeMapPreviewSplit .": preview split\n"
+        let help .= '" '. g:NERDTreeMapOpenVSplit .": open vsplit\n"
+        let help .= '" '. g:NERDTreeMapPreviewVSplit .": preview vsplit\n"
         let help .= '" '. g:NERDTreeMapCustomOpen .": custom open\n"
         let help .= '" '. g:NERDTreeMapDeleteBookmark .": delete bookmark\n"
 
@@ -364,7 +368,7 @@ function! s:UI.saveScreenState()
         call g:NERDTree.CursorToTreeWin()
         let self._screenState['oldPos'] = getpos('.')
         let self._screenState['oldTopLine'] = line('w0')
-        let self._screenState['oldWindowSize']= winwidth('')
+        let self._screenState['oldWindowSize'] = winnr('$')==1 ? g:NERDTreeWinSize : winwidth('')
         call nerdtree#exec(win . 'wincmd w', 1)
     catch
     endtry

nerdtree_plugin/fs_menu.vim

@@ -165,7 +165,7 @@ endfunction
 function! NERDTreeAddNode()
     let curDirNode = g:NERDTreeDirNode.GetSelected()
     let prompt = s:inputPrompt('add')
-    let newNodeName = input(prompt, curDirNode.path.str() . g:NERDTreePath.Slash(), 'file')
+    let newNodeName = input(prompt, curDirNode.path.str() . nerdtree#slash(), 'file')
 
     if newNodeName ==# ''
         call nerdtree#echo('Node Creation Aborted.')
@@ -252,8 +252,6 @@ endfunction
 
 " FUNCTION: NERDTreeDeleteNode() {{{1
 function! NERDTreeDeleteNode()
-    let l:shellslash = &shellslash
-    let &shellslash = 0
     let currentNode = g:NERDTreeFileNode.GetSelected()
     let confirmed = 0
 
@@ -289,7 +287,6 @@ function! NERDTreeDeleteNode()
     else
         call nerdtree#echo('delete aborted')
     endif
-    let &shellslash = l:shellslash
 endfunction
 
 " FUNCTION: NERDTreeListNode() {{{1
@@ -334,8 +331,6 @@ endfunction
 
 " FUNCTION: NERDTreeCopyNode() {{{1
 function! NERDTreeCopyNode()
-    let l:shellslash = &shellslash
-    let &shellslash = 0
     let currentNode = g:NERDTreeFileNode.GetSelected()
     let prompt = s:inputPrompt('copy')
     let newNodePath = input(prompt, currentNode.path.str(), 'file')
@@ -371,7 +366,6 @@ function! NERDTreeCopyNode()
     else
         call nerdtree#echo('Copy aborted.')
     endif
-    let &shellslash = l:shellslash
     redraw!
 endfunction
 

syntax/nerdtree.vim

@@ -47,7 +47,7 @@ endif
 "highlighting for readonly files
 exec 'syn match NERDTreeRO #.*'.g:NERDTreeNodeDelimiter.'\zs.*\ze'.g:NERDTreeNodeDelimiter.'.*\['.g:NERDTreeGlyphReadOnly.'\]# contains=NERDTreeIgnore,NERDTreeBookmark,NERDTreeFile'
 
-exec 'syn match NERDTreeFlags #\[[^\]]*\]\ze'.g:NERDTreeNodeDelimiter.'# containedin=NERDTreeFile,NERDTreeExecFile,NERDTreeDir'
+exec 'syn match NERDTreeFlags #\[[^\]]*\]\ze'.g:NERDTreeNodeDelimiter.'# containedin=NERDTreeFile,NERDTreeExecFile,NERDTreeLinkFile,NERDTreeRO,NERDTreeDir'
 
 syn match NERDTreeCWD #^[</].*$#
 
@@ -93,3 +93,5 @@ hi def link NERDTreeBookmark Statement
 hi def link NERDTreeFlags Number
 
 hi def link NERDTreeCurrentNode Search
+
+hi NERDTreeFile ctermbg=NONE guibg=NONE