robcowie / SublimeTODO

**[DEPRECATED]** - See https://github.com/jonathandelgado/SublimeTodoReview - Extract TODO-type comments from open files and project folders
295 stars 54 forks source link

Hangs on 'Finding TODOs' #1

Closed jaylevitt closed 12 years ago

jaylevitt commented 12 years ago

The status line keeps throbbing but it never comes back. No output in the console; where should I look for errors? I'm on Mac OS X 10.6.8.

robcowie commented 12 years ago

Er, yeah. I've conveniently ignored error logging so far. Few questions;

I'll see what I can come up with.

jaylevitt commented 12 years ago

1 open file, 1805 in project, 4319 files in pwd. Might be some binaries in there that are catching it; it doesn't look like it skips binary files, at least not using ST's binary list. Maybe pss 0.34 with its --textonly would help?

robcowie commented 12 years ago

I can run it successfully with a large repo open in the sidebar (many nested dirs, ~10k files, including small binaries). It takes about 2 minutes and lists about 3000 todo items.

However, I'll spend some time tomorrow (it's late here) to do the following:

Thanks for letting me know about the issue btw, I appreciate the feedback.

jaylevitt commented 12 years ago

Thanks - watching how you do that should give me enough Python-fu to narrow down the problem some more..

robcowie commented 12 years ago

There is a horribly inefficient 'hack' to account for the fact that pss returns the entire matching line, and does not support regexp groups. Usually, reporting the entire line is desired, but in this case we only want the matching TODO comment. A custom formatter runs the pattern on the matching line a second time to extract the comment.

I'll see if I can get a pull request sorted to add support for regexp groups in the pss MatchResult object. That will allow me to simplify the code a bit.

A bigger issue is the fact that the files are scanned once per pattern. I haven't found a way around that yet though.

jaylevitt commented 12 years ago

Oof. Welll.. do you really need the full power of pss for this? Maybe launching ack in a subprocess isn't so horrible after all...

robcowie commented 12 years ago

Same problem regardless of how the files are searched; There are several patterns, and the output should be grouped by 'comment type' (i.e. todo, fixme etc.). If the multiple patterns are joined to form one ('TODO|FIXME|CHANGED') then only one pass will be required, but the results will not be grouped.

This is probably a better solution; grep the files once, sort and group the result strings in memory, then render. I'll look into it and do some perf testing.

Also, the issue with scanning a matched line a second time has been resolved. The pas MatchResult() object contains column ranges that can be used to slice the string. Sorted.

robcowie commented 12 years ago

I've pushed some changes;

jaylevitt commented 12 years ago

ah, ok - the "hang" is pss parsing my very long log files, which are not part of the project but which are apparently getting included anyway by virtue of being in a child of the current dir. I wonder if there's a way to get ST's idea of "files that should be searchable", which would incorporate its folder and file patterns..

robcowie commented 12 years ago

Interesting. I'm not currently aware of a way to get a list of files in the current project. window.folders() returns paths to all folders currently in the sidebar. The plugin scans these folders, and any other open documents.

ST defines file_exclude_patterns to exclude files from the sidebar but by default that does not exclude .log files. Now I think about it though, excluding them probably is a reasonable default for the plugin; I can't imagine many people extracting TODO comments from their logs.

Should be possible to extend that by supporting per-project settings for excluding files.

Perhaps the plugin should exclude ST file_exclude_patterns, the standard pas excluded files and selected other files such as .log.

jaylevitt commented 12 years ago

To clarify, I have the following in my user's Global.sublime-settings:

"folder_exclude_patterns": [".svn", ".git", ".hg", "CVS", "tmp", "log", "development", "public", "vendor", "images"],

"file_exclude_patterns": ["*.pyc", "*.pyo", "*.exe", "*.dll", "*.obj","*.o", "*.a", "*.lib", "*.so", "*.dylib",
    "*.ncb", "*.sdf", "*.suo", "*.pdb", "*.idb", ".DS_Store", "*.class", "*.psd", "*.db", ".*", "dbdump.dat",
    "*.sublime-*", "*.tmproj", "*.sqlite3"],

"binary_file_patterns": ["*.jpg", "*.jpeg", "*.png", "*.gif", "*.ttf", "*.tga", "*.dds", "*.ico", "*.eot", "*.pdf", "*.swf", "*.jar", "*.zip", "*.min.js", "*.pack.js"]

So if you are able to exclude file_exclude_patterns, folder_exclude_patterns and binary_file_patterns, you'll behave the same way find-in-files does. (The first two patterns are for the sidebar, and the last is additional exclusions that should show up in the sidebar but be skipped by search.)

Because I have log in folder_exclude_patterns, it doesn't show up in the sidebar - but it is a subdirectory of my project folder, which of course does show up, so if you're scanning all subdirectories and not excluding anything, that'd explain it.

robcowie commented 12 years ago

folder_exclude_patterns, file_exclude_patterns and binary_file_patterns are now honoured. I'd appreciate your comments.

The code is getting a bit convoluted now. No biggie - this is basically a work in progress. A cleanup is in order I think.

jaylevitt commented 12 years ago

Hm, not soup yet:

Package Control: Installing 1 upgrades
Package Control: Upgraded SublimeTODO to v2011.11.25.15.51.05
Reloading plugin /Users/jay/Library/Application Support/Sublime Text 2/Packages/SublimeTODO/results_formatter.py
Reloading plugin /Users/jay/Library/Application Support/Sublime Text 2/Packages/SublimeTODO/todo.py
error: Package Control: There are no packages ready for upgrade.
fetching global settings
Exception in thread Thread-130:
Traceback (most recent call last):
  File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/threading.py", line 522, in __bootstrap_inner
    self.run()
  File "./todo.py", line 167, in run
  File "./todo.py", line 117, in extract
TypeError: pss_run() got an unexpected keyword argument 'add_ignored_files'
robcowie commented 12 years ago

That's odd. The new keyword arg was definitely added to pss_run() in the last commit. See 10ddb9df9831dfe31b32636e43601a8c629bbf8f. It might be something weird going on with package control, though I can't think what. Perhaps try removing the plugin dir and reinstalling it.

robcowie commented 12 years ago

I've replaced pss with some simpler code that combines patterns and iterates files once, rather than once per pattern as dictated by pss. I probably should modify pss and submit a pull request, but in the short term, this is a much quicker improvement.

So, plenty has changed. It would be great if you can update, and let me know if you encounter any more problems.

cheers and Merry Christmas, Rob Cowie

jaylevitt commented 12 years ago

YES! It works beautifully now. Thanks!