Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added support for limiting ag search to file types #40

Merged
merged 5 commits into from Jun 7, 2016

Conversation

anuragpeshne
Copy link
Contributor

while search for a certain function in a Python file, dumb-jump got me to function with same name in JavaScript file. Hence I added support for limiting search to file types which should speed up searching too.

Corrected the way ag file types are concatenated.

@coveralls
Copy link

coveralls commented May 27, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 5b34258 on anuragpeshne:master into 0edf267 on jacktasia:master.

@anuragpeshne
Copy link
Contributor Author

The code is failing just one test case
dumb-jump-generate-ag-command-no-ctx-test
Probably because of the extra whitespace added in the cmd generated

@jacktasia
Copy link
Owner

jacktasia commented May 27, 2016

Thanks for the PR! This is an interesting idea. Have you noticed an increased jump speed?

It looks like the dumb-jump-generate-ag-command-no-ctx-test test is failing because it hasn't been updated to to include the --elisp argument. Additionally, it seems like you'll want to wrap agtypes with -distinct so it doesn't show up more than once.

image

@coveralls
Copy link

coveralls commented May 29, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 0299292 on anuragpeshne:master into 0edf267 on jacktasia:master.

@anuragpeshne
Copy link
Contributor Author

adding -distinct did the trick! thanks.
I have also updated the test file to expect ag-types.

I tried benchmarking the scripts but couldn't notice anything remarkable, maybe it will be prominent in a large folder.

@jacktasia
Copy link
Owner

Awesome, thanks for fixing that test. Looks like there's just there's just a tiny merge conflict now. Should be trivial to fix because I think it happened when I resolved #38, which was just adding two new clojure extensions. Thanks again!

@scottaj
Copy link

scottaj commented Jun 1, 2016

I use dumb jump with JavaScript and AngularJS. One of the best things about it is that if I use dumb jump on a variable or function in an HTML template, it will find it in the associated JS file, which Tern cannot do. Will this change break that functionality?

@jacktasia
Copy link
Owner

@scottaj Fantastic point. Thank you so much for bringing this up. Based on this and this. I think this PR would indeed break the functionality you're talking about. So @anuragpeshne I think the easiest solution is to wrap this in a flag like dumb-jump-use-agtypes that defaults to nil. This way if someone has a huge non-mixed language project they can easily turn on this functionality for faster jumping, but it won't hurt people who depend on the current functionality.

(defcustom dumb-jump-use-agtypes
  nil
  "When enabled dumb-jump will only search the same filetypes as the starting file."
  :group 'dumb-jump)

Thanks again!

@coveralls
Copy link

coveralls commented Jun 2, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling 3bf1e25 on anuragpeshne:master into 590773b on jacktasia:master.

@anuragpeshne
Copy link
Contributor Author

anuragpeshne commented Jun 2, 2016

@scottaj I had this case in my mind while limiting the search and hence we pass list of file types to ag. In case of Javascript we pass both --js and --html. Since html is associated with Javascript, you can search in html template and it will search in other html files as well as JS files.
RUNNING CMD '"ag --nocolor --nogroup --js --html \"(service|factory)\\((['...
This would not work if there are more than one language in a single extension (example: PHP emitting Javascript). But that would anyways won't work, since we'll be using PHP's regex to grep JS variables/functions. If there are any other cases, please let me know.

@jacktasia There was a small bug while extracting agtype from dumb-jump-language-file-exts which is now resolved. Also, I've added agtype for scala which now resolves the conflict with base branch.

@jacktasia
Copy link
Owner

@anuragpeshne Awesome, thanks! Let me play with this a bit before I merge it.

@jacktasia
Copy link
Owner

@anuragpeshne Looks good to me. Thanks again!

@scottaj I tested this PR jumping from HTML to javascript and it worked just fine. Please open an issue if you notice anything. Thanks again!

@jacktasia jacktasia merged commit d47d9c1 into jacktasia:master Jun 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants