Refactor GitHub Code Search, and provide a non-repository scoped version [githubcodesearch] #10687
Conversation
|
ccoVeille
force-pushed
the
github-code-search
branch
from
8fbca88
to
99ef02e
Compare
ccoVeille
force-pushed
the
github-code-search
branch
from
99ef02e
to
0cf4a48
Compare
|
It's ready for review. Thanks |
jNullj
changed the title
jNullj
changed the title
jNullj
changed the title
| }, | ||
| } | ||
|
|
||
| async handle({ user, repo, query }) { |
There was a problem hiding this comment.
We can remove these as they are unused
| async handle({ user, repo, query }) { | |
| async handle({ query }) { |
| static route = { | ||
| base: 'github/search', | ||
| pattern: ':query+', | ||
| } |
There was a problem hiding this comment.
This will not work with the route of GitHubRepoCodeSearch as this includes all paths of the GitHubRepoCodeSearch in it.
If you make a request to http://localhost/github/search/torvalds/linux/goto it calls GitHubCodeSearch and not GitHubRepoCodeSearch.
Please change the route
There was a problem hiding this comment.
Good catch on this issue.
In terms of how to solve it..
There's no way to distinguish between /:user/:repo/:query+ and a /:query+ that contains multiple slash characters.
One way round this would be to put the non repo-scoped version on a different route (so we pick /github/global-search/:query+ or something?). However, I think the optimal way to solve this would be to match what we've done with the GitHub Issue Search badge and make query a QueryString param instead of a path param.
https://shields.io/badges/git-hub-issue-custom-search
https://shields.io/badges/git-hub-issue-custom-search-in-repo
That way they could be /github/search?query=foo and /github/search/user/repo?query=foo, and query can contain any number of slashes. It would also make the URL format match up with this existing conceptually similar badge.
In order to do this, we'd also have to add a redirector for /github/search/:user/:repo/:query+ which converts query to a QueryString param to avoid breaking existing badges. There is an example of how to do this at https://github.com/badges/shields/blob/master/services/gitlab/gitlab-coverage-redirect.service.js - this redirect switches branch to a query param.
There was a problem hiding this comment.
Thank you for your actionable feedbacks. I'll take a look.
Stay tuned
Fixes #10686