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

add support for multiple extension filters #2019

Merged
merged 2 commits into from
Oct 3, 2019
Merged

add support for multiple extension filters #2019

merged 2 commits into from
Oct 3, 2019

Conversation

ecarlson94
Copy link
Contributor

Modifies SearchCodeRequest.Extension to be a list for filtering by x number of extensions. Helps reduce the number of required API calls.

Question: Is there an open issue to support the negation of qualifiers (-extension:{0})? I couldn't find one.

@codecov
Copy link

codecov bot commented Sep 30, 2019

Codecov Report

Merging #2019 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2019      +/-   ##
==========================================
+ Coverage   70.77%   70.77%   +<.01%     
==========================================
  Files         535      535              
  Lines       14062    14063       +1     
==========================================
+ Hits         9952     9953       +1     
  Misses       4110     4110
Impacted Files Coverage Δ
Octokit/Models/Request/SearchCodeRequest.cs 92.72% <100%> (+0.13%) ⬆️

@shiftkey
Copy link
Member

shiftkey commented Oct 1, 2019

Question: Is there an open issue to support the negation of qualifiers (-extension:{0})? I couldn't find one.

@ecarlson94 not that I'm aware of. Please open one if this is something you'd like to see it supported.

@shiftkey
Copy link
Member

shiftkey commented Oct 1, 2019

@ecarlson94 thanks for opening this up. I also know there's a docs example in docs/search.md that will need to be updated to support this change:

var request = new SearchCodeRequest("auth")
{
    // we can restrict search to the file, path or search both
    In = new[] { CodeInQualifier.File, CodeInQualifier.Path },
    
    // how about we find a file based on a certain language
    Language = Language.JavaScript,
    
    // do we want to search forks too?
    Forks = true,

    // find files that are above 1000 bytes
    Size = Range.GreaterThan(1000),

    // we may want to restrict the search to the path of a file
    Path = "app/assets",
    
    // we may want to restrict the file based on file extension
    Extension = "json",
    
    // restrict search to a specific file name
    FileName = "app.json",
    
    // search within a users or orgs repo
    User = "dhh"
};

@ecarlson94
Copy link
Contributor Author

@shiftkey, thanks for the quick response. I updated the documentation. Sorry I missed it.

I also changed the Extensions property from an IList to an IEnumerable. It matches the conventions already in place. And since SearchCodeRequest is an object that has to be manually create, users won't need IList.Add() when they can just statically initialize the property.

@ecarlson94
Copy link
Contributor Author

ecarlson94 commented Oct 2, 2019

@shiftkey, any idea what I should do about this Travis CI error.

@shiftkey
Copy link
Member

shiftkey commented Oct 3, 2019

@ecarlson94 it's unrelated to the PR, so I've just re-run it to see if it'll pass

@shiftkey
Copy link
Member

shiftkey commented Oct 3, 2019

release_notes: SearchCodeRequest.Extension has been renamed to SearchCodeRequest.Extensions to support multiple file extensions

@shiftkey shiftkey merged commit 3e7c70c into octokit:master Oct 3, 2019
@ecarlson94 ecarlson94 deleted the multiple-extensions branch October 3, 2019 16:16
@nickfloyd nickfloyd added Type: Bug Something isn't working as documented and removed category: bug labels Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working as documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants