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 more options #49

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Added more options #49

wants to merge 6 commits into from

Conversation

derekwyatt
Copy link

  • 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

Derek Wyatt and others added 4 commits February 22, 2014 16:20
- 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
- If the current file's directory is the same as the one containing the scm
  directory, then we fail to return '.'
@losingkeys
Copy link
Collaborator

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:

@derekwyatt
Copy link
Author

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.

@losingkeys
Copy link
Collaborator

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 losingkeys closed this Feb 26, 2014
@losingkeys losingkeys reopened this Feb 26, 2014
@losingkeys
Copy link
Collaborator

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!

ches referenced this pull request in mileszs/ack.vim Mar 24, 2014
@ches
Copy link

ches commented Mar 24, 2014

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...

Numkil pushed a commit to Numkil/ag.nvim that referenced this pull request Dec 18, 2015
- Also separated testcase for partial/whole match
- AgGroup tests wasn't touched

Closed rking#29
Ref rking#49
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

3 participants