A few minor changes were made to the "Bookmark.getNode()" function for
the purposes of improving readability and documentation clarity.
This process also led me to the conclusion that the "findNode()"
function should be refactored to throw an error if a node cannot be
found. This would lead to greater uniformity in the reporting of
failures to find a node. It is generally better style to have an error
thrown as close to the source as possible. A substantial change like
this should wait for now.
A missing argument sigil is effectively a syntax error in VimL. The
function in which the error occurred was called in the execution of at
least three buffer-local NERDTree commands:
1. :Bookmark (specifically, when trying to overwrite a Bookmark)
2. :OpenBookmark
3. :RevealBookmark
Only one specific type of error message associated with these commands
is fixed here (see issue #677).
The problems with the above commands are not fully addressed by this
commit, and their behavior can be improved immensely by further
refactoring. However, no one has been able to even use these commands at
all before now because the fix given here was not in place.
More work will need to be done to improve the behavior of these commands
so that they truly function as any reasonable user would expect.
Fixes#677.
The header in "bookmark.vim" was pretty weak. It provided no description
of the class it contains and no direction for the reader. In particular
it did not note the dual purpose of the "Bookmark" class.
The fact that the "Bookmark" class serves two purposes must be noted
because many readers will expect class definitions to obey the single
responsibility principle!
If there is a chance for a major refactor of this class in the future, a
priority would be splitting the class in two so that a "BookmarkList"
class can assume the responsibility for providing a container for all
"Bookmark" objects.
The function in "bookmark.vim" that allows the caller to query the list
of Bookmarks by name had stale commentary. In addition, the internals of
the function needed to be reworked to improve readability. Making this
function very clean is important because it is heavily used elsewhere.
As a side note, it might be beneficial to later rename this function to
something like "GetBookmarkByName" to further improve readability. That
change is not critical and can be safely delayed.
It makes the most sense to sort the global bookmarks list just before
rendering them in the NERDTree window. Since Vim's sort function is fast
and stable, and since users are very unlikely to have a number of
bookmarks that is too large, we can sort before rendering without
concern for the negligible performance penalty.
This has two benefits:
1. Users can change their sort settings and have them take effect
on the next render or refresh.
2. As mentioned, code duplication is avoided.
Sorting the list of user bookmarks requires care to ensure that Vim's
builtin sort function is called correctly. Previously, this function was
called incorrectly. This is why the sorting of bookmarks never worked.
The offending functions have been removed here and replaced with
"s:Bookmark.CompareBookmarksByName". To understand the necessity for
this change, read ":h sort()" for the requirements of the function
reference argument (esp., note that it must return -1, 0, or 1).
In addition to fixing this problem, the new comparison function will
inspect the "g:NERDTreeBookmarksSort" setting to determine whether
case-sensitivity is preferred in the sort. The documentation has been
modified to accurately reflect this adjustment. The change is also made
in such a way as not to break any existing configurations.
Fixes#361 ("My bookmarks aren't sorted").
The trailing fold markers in "bookmark.vim" varied in how far they were
from the end of the line. This created an unpleasant visual effect when
folding was in use.
Wildignore rules that involve directories aren't handled properly (or at
least as one might expect) when globpath uses absolute paths. For
example, when wildignore contains `src/*.js`, `globpath("src", "*")`
works as expected, while `globpath("/home/user/project/src", "*")`
doesn't.
So :bprev was failing to jump back to the right buffer. I dont fully
understand this yet, but I'm fairly sure this is because there is a
magic <directory> buffer that we delete when creating a wintree :-/
Anyway, we are explicitly storing the prev buffer again - and updating
it when reusing treewins. It's worky, but there may be a superior way...
Some will have a "NERDTree" buffer variable and others will not. In the
ones that do, getbufvar will return a dictionary. getbufvar will return
an empty string from the other buffers. When looping through the buffers,
let will throw an error if nt already exists and is a different type than
what is trying to be put into it. This easily can be illustrated by these
two statements:
:let x = ""
:let x = {}
E706: Variable type mismatch for: x
This commit gets rid of the variable before setting it so that the
mismatch cannot occur.
