Comments
-
Greg Hurrell
Any idea of what the performance impact of this is on really large directory hierarchies? (ie. crank up the scan depth and max file limit and then see how it performs on an enormous hierarchy.)
-
Greg Hurrell
I've had a bit of a play around with this and I like the basic idea of it. It seems to handle most of the usage scenarios mentioned in ticket #1542:
-
existing simple file globs like
*.oand.git - patterns with leading
**
Notably it doesn't handle patterns of the form
vendor/rails/**like the person who posted that ticket was originally requesting. This is because internally, we're working with absolute paths at that point and/absolute/path/to/vendor/rails/foois not considered by VIM as a match. Instead, you would need to specify a pattern like**/vendor/rails/**.One change required, however. It appears that on at least some versions of VIM the Ruby support doesn't know how to turn "NULL" results into an empty string, which means that you'll get a "NULL pointer given" exception raised every time you test an excluded path. From Googling I see that this isn't something that happens to affect just the platform I'm using (MacVim).
So this is the tweaked version I'm testing, which uses
empty(exclude('...'))rather than just a nakedexclude('...'):diff --git a/ruby/command-t/controller.rb b/ruby/command-t/controller.rb index 51277ce..3b93c19 100644 --- a/ruby/command-t/controller.rb +++ b/ruby/command-t/controller.rb @@ -148,8 +148,7 @@ module CommandT :max_depth => get_number('g:CommandTMaxDepth'), :always_show_dot_files => get_bool('g:CommandTAlwaysShowDotFiles'), :never_show_dot_files => get_bool('g:CommandTNeverShowDotFiles'), - :scan_dot_directories => get_bool('g:CommandTScanDotDirectories'), - :excludes => get_string('&wildignore') + :scan_dot_directories => get_bool('g:CommandTScanDotDirectories') end def get_number name diff --git a/ruby/command-t/scanner.rb b/ruby/command-t/scanner.rb index a48d070..c773d72 100644 --- a/ruby/command-t/scanner.rb +++ b/ruby/command-t/scanner.rb @@ -31,7 +31,6 @@ module CommandT @max_depth = options[:max_depth] || 15 @max_files = options[:max_files] || 10_000 @scan_dot_directories = options[:scan_dot_directories] || false - @excludes = (options[:excludes] || '*.o,*.obj,.git').split(',') end def paths @@ -61,16 +60,15 @@ module CommandT private def path_excluded? path - @excludes.any? do |pattern| - File.fnmatch pattern, path, File::FNM_DOTMATCH - end + path = Vim.escape_for_single_quotes path + VIM.evaluate("empty(expand('#{path}'))").to_i == 1 end def add_paths_for_directory dir, accumulator Dir.foreach(dir) do |entry| next if ['.', '..'].include?(entry) path = File.join(dir, entry) - unless path_excluded?(entry) + unless path_excluded?(path) if File.file?(path) @files += 1 raise FileLimitExceeded if @files > @max_files -
existing simple file globs like
-
Greg Hurrell
And here's my entry into the commit log essay contest:
commit bbf296b775d5d9220fae28a34111117312bfcaa7 Author: Greg Hurrell <greg@hurrell.net> Date: Tue May 11 21:56:57 2010 +0200 Bring wildignore behavior closer to VIM's own behaviour In commit f314c1e6 we tried to make Command-T react to the 'wildignore' setting in way that more closely matches VIM's own behaviour by using the builtin expand() function which takes into account 'wildignore'. While this works quite well, there are still some discrepancies from VIM's actual behaviour. For example, while patterns like this work as expected with no surprises: *.o .git **/build A pattern like this: vendor/rails/** Will work differently in Command-T than it does in the rest of VIM. Specifically, if I type this in VIM: :e vendor/rails/<tab> Then VIM's autocomplete won't allow me to drill down into the directory because it is excluded by the 'wildignore' setting. Command-T, on the other hand, will allow me to see the contents of that directory. This is because internally, at the time the file names are checked, they are almost always absolute paths, because the default starting directory is ":pwd" which is itself provided by VIM as an absolute path. As such, VIM's expand() function checks to see if: /absolute/path/to/vendor/rails/ Matches against the 'wildignore' pattern of: vendor/rails/** And decides that it does not match. As a work around, a user could specify a pattern like this: **/vendor/rails/** But it is a bit ugly because it doesn't fit well with the behaviour of VIM itself. The fix, then, is to not pass absolute paths into the expand() function, but to pass paths relative to the starting directory. In 99% of cases, the starting directory is the ":pwd", so the behaviour should then be identical to VIM's own behaviour. In the cases where the user has passed in an override for the starting directory (either relative or absolute), then the behaviour will diverge slightly from VIM's while still hopefully being consistent and intuitive from the user's perspective. For example, if :pwd is the HOME directory and the user invokes Command-T with: :CommandT path/to/some/rails/project Then a 'wildignore' which includes this pattern: vendor/rails/** Will exclude all files under: path/to/some/rails/prokect/vendor/rails/ Which is probably what the user expects. For comparison, typing: :e path/to/some/rails/project/vendor/rails/<tab> Will autocomplete despite the 'wildignore' settings, which is consistent with the standard VIM behaviour as described previously. Signed-off-by: Greg Hurrell <greg@hurrell.net> diff --git a/ruby/command-t/scanner.rb b/ruby/command-t/scanner.rb index c773d72..1868e95 100644 --- a/ruby/command-t/scanner.rb +++ b/ruby/command-t/scanner.rb @@ -21,13 +21,15 @@ # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. +require 'pathname' + module CommandT # Reads the current directory recursively for the paths to all regular files. class Scanner class FileLimitExceeded < ::RuntimeError; end def initialize path = Dir.pwd, options = {} - @path = path + @path = path ? Pathname.new(path) : nil @max_depth = options[:max_depth] || 15 @max_files = options[:max_files] || 10_000 @scan_dot_directories = options[:scan_dot_directories] || false @@ -39,7 +41,7 @@ module CommandT @paths = [] @depth = 0 @files = 0 - @prefix_len = @path.chomp('/').length + @prefix_len = @path.to_s.chomp('/').length add_paths_for_directory @path, @paths rescue FileLimitExceeded end @@ -51,8 +53,9 @@ module CommandT end def path= str - if @path != str - @path = str + pathname = str ? Pathname.new(str) : nil + if @path != pathname + @path = pathname flush end end @@ -60,6 +63,7 @@ module CommandT private def path_excluded? path + path = path.relative_path_from(@path).to_s path = Vim.escape_for_single_quotes path VIM.evaluate("empty(expand('#{path}'))").to_i == 1 end @@ -67,13 +71,13 @@ module CommandT def add_paths_for_directory dir, accumulator Dir.foreach(dir) do |entry| next if ['.', '..'].include?(entry) - path = File.join(dir, entry) + path = dir + entry unless path_excluded?(path) - if File.file?(path) + if path.file? @files += 1 raise FileLimitExceeded if @files > @max_files - accumulator << path[@prefix_len + 1..-1] - elsif File.directory?(path) + accumulator << path.to_s[@prefix_len + 1..-1] + elsif path.directory? next if @depth >= @max_depth next if (entry.match(/\A\./) && !@scan_dot_directories) @depth += 1 -
anonymous
I'm going to rethink this a bit (going through VIM:: makes it rather hard to test).
-
anonymous
oops, ignore my last post (and this one). I forgot to reload the page before I posted it.
-
Greg Hurrell
Performance of the Pathname-based code is a bit sucky, based on my subjective testing. Gonna try replacing it with something simpler.
-
Greg Hurrell
This replaces the Pathname-based code with a much faster "inline" alternative:
diff --git a/ruby/command-t/scanner.rb b/ruby/command-t/scanner.rb index 1868e95..c14668d 100644 --- a/ruby/command-t/scanner.rb +++ b/ruby/command-t/scanner.rb @@ -21,15 +21,13 @@ # ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. -require 'pathname' - module CommandT # Reads the current directory recursively for the paths to all regular files. class Scanner class FileLimitExceeded < ::RuntimeError; end def initialize path = Dir.pwd, options = {} - @path = path ? Pathname.new(path) : nil + @path = path @max_depth = options[:max_depth] || 15 @max_files = options[:max_files] || 10_000 @scan_dot_directories = options[:scan_dot_directories] || false @@ -41,7 +39,7 @@ module CommandT @paths = [] @depth = 0 @files = 0 - @prefix_len = @path.to_s.chomp('/').length + @prefix_len = @path.chomp('/').length add_paths_for_directory @path, @paths rescue FileLimitExceeded end @@ -53,9 +51,8 @@ module CommandT end def path= str - pathname = str ? Pathname.new(str) : nil - if @path != pathname - @path = pathname + if @path != str + @path = str flush end end @@ -63,7 +60,8 @@ module CommandT private def path_excluded? path - path = path.relative_path_from(@path).to_s + # first strip common prefix (@path) from path + path = path[(@prefix_len + 1)..-1] path = Vim.escape_for_single_quotes path VIM.evaluate("empty(expand('#{path}'))").to_i == 1 end @@ -71,13 +69,13 @@ module CommandT def add_paths_for_directory dir, accumulator Dir.foreach(dir) do |entry| next if ['.', '..'].include?(entry) - path = dir + entry + path = File.join(dir, entry) unless path_excluded?(path) - if path.file? + if File.file?(path) @files += 1 raise FileLimitExceeded if @files > @max_files - accumulator << path.to_s[@prefix_len + 1..-1] - elsif path.directory? + accumulator << path[@prefix_len + 1..-1] + elsif File.directory?(path) next if @depth >= @max_depth next if (entry.match(/\A\./) && !@scan_dot_directories) @depth += 1 -
Greg Hurrell
Ok, I've now done enough testing of this that it's at the point where I feel ok with pushing it out.
Still not at the point where I feel comfortable cutting a release, but at least it's out there in the public eye now.
-
Greg Hurrell
Status changed:
- From: new
- To: closed
Add a comment
Comments are now closed for this issue.