ag.vim icon indicating copy to clipboard operation
ag.vim copied to clipboard

Added more options

Open derekwyatt opened this issue 11 years ago • 5 comments

  • There are now a few options that you can specify
    • current_file_ext -- limits the search to files that have the current buffer's file's extension only
    • current_file_dir -- limits the search to files that are in the current buffer's file's directory
    • scmdir -- starts the search at the "root" of a source directory, based on the existence of a .git, .scm or .hg dir
    • specific_dirs -- limits the search to these directories
  • You can also customize the quickfix mappings
  • Each mapping is customizable using a dictionary

derekwyatt avatar Feb 22 '14 21:02 derekwyatt

Wow! This is quite the pull request. Thanks for taking the time to do this. It looks like this'll make mappings a lot more useful in addition to the extra commands.

Running :Ag blah using the latest (a835c0132bb49935ef9bab226e866ede942d505c) locally gives me an error: E119: Not enough arguments for function: ag#Ag.

Some other small things I noticed:

  • Rather than checking each key of the g:ag_results_mapping, you could have a variable w/the defaults, then merge the two (preferring the users keys if they're set), for example: https://github.com/losingkeys/vim-stopsign/blob/ace34e65209fd233cafee657668de7a46bc41dba/plugin/stopsign.vim#L10-L26 . This isn't really a big deal, and definitely not a merge bocker, but might make for smaller diffs down the road
  • The code examples in the docs sometimes have extra lines. Again not a merge blocker, just a small thing :)

losingkeys avatar Feb 25 '14 01:02 losingkeys

Ah, I didn't realize you wanted everything to actually work :)

So, :Ag should be fine now (as should everything else, I believe). I addressed the dictionary extension, which is a lot better and easier to read. I'm not sure what the 'extra lines' in the docs is about, but I'm sure you can pretty that up later.

derekwyatt avatar Feb 25 '14 13:02 derekwyatt

Nice, that was some quick work. I still want to play with it a little more, just to be sure I don't miss anything (I often miss stuff in my PRs, so I'm trying to be more thorough these days).

I think you can search specific filetypes with something like :Ag term **/*.{js,rb,whatever}, but I don't think that completely negates the need for :AgForExtension, as that's probably more convenient.

losingkeys avatar Feb 26 '14 15:02 losingkeys

Ok, so I've been taking another look at this, and it looks like it's just about ready to be merged. I just have a couple more questions/nits to pick if that's alright.

Because of the way the other commands are named (:AgFile for example), you probably don't need the extra 'For' in the new commands (so :AgForExtension could become :AgExtension, without (imo) too much confusion). I'll leave this up to you, it was just a last-minute observation on my part.

The documentation around :AgForExtension doesn't need the dots, because it's looking at the suffixes (so it could just be AgForExtension 'search for me' js java txt). I realize this is a really small thing, but it's something I wouldn't have noticed before, and it might speed up users' use of this plugin :).

The documentation around :AgForCurrentFileDir makes it sound like it searches only the files in the directory of whatever window is focused right now, or the :pwd if you have a blank window open. Testing it out it always seemed to be the current directory for me. Is it supposed to work differently? or should the documentation be changed?

I had the same issue with :AgForProjectRoot; it didn't seem to search the closest parent with a .git directory, but the PWD instead.

Any other issues I found weren't an issue with this pull request (they're either tracked already or possible enhancements), so this is the last bit of stuff I found. Let me know if any of this is confusing, and thanks again for the code!

losingkeys avatar Mar 07 '14 15:03 losingkeys

Worth some consideration if, in the long run, the Ack-like file types support (ggreer/the_silver_searcher#283) might be preferable to file extensions with the -G option. But I have my doubts that they'll ever have the configuration flexibility that Ack does in this regard -- the author has said it's not a feature he used much, and there is as yet no configuration mechanism for ag at all...

ches avatar Mar 24 '14 00:03 ches