Conversation
| return { | ||
| url: `search?q=${encodeURIComponent(query)}&sq=repo:%5E${encodeURIComponent( | ||
| repoPath.replace(/\./g, '\\.') | ||
| )}%24@${encodeURIComponent(rev)}&utm_source=${getPlatformName()}`, |
There was a problem hiding this comment.
I would use the URL API to avoid manual encodeURIComponent calls
src/libs/code_intelligence/search.ts
Outdated
| if (checkIsSearchPage()) { | ||
| storage.getSync(({ executeSearchEnabled }) => { | ||
| // GitHub search page pathname is <org>/<repo>/search | ||
| if (false && !executeSearchEnabled) { |
There was a problem hiding this comment.
Lol good catch. Was too lazy to switch the feature flag while working
| const buildURL = (owner: string, repoName: string, path: string) => | ||
| `${window.location.origin}/api/v4/projects/${owner}%2f${repoName}${path}` | ||
|
|
||
| const get = <T>(url: string): Observable<T> => ajax.get(url).pipe(map(({ response }) => response as T)) |
There was a problem hiding this comment.
Avoid type parameters that are only used in the return type - it's only a cast in disguise. It's better to cast explicitly.
There was a problem hiding this comment.
Casting is actually what I intended with that. I think it's easier to read at the call sights.
src/libs/gitlab/scrape.ts
Outdated
| const repoName = parts[1] | ||
|
|
||
| let pageKind: GitLabPageKind | ||
| if (window.location.pathname.match(new RegExp(`${owner}/${repoName}/commit`))) { |
There was a problem hiding this comment.
Wouldn't pathname.includes() do the same? Do you have to escape characters in the regex?
| } | ||
| } | ||
|
|
||
| const createErrorBuilder = (message: string) => (kind: string) => new Error(`${message} (${kind})`) |
There was a problem hiding this comment.
What is the purpose of kind? It looks more like an error code than meant for human consumption, but it is put into the message.
There was a problem hiding this comment.
It's added for making it easier to find the site of the error. Maybe kind is just a bad name.
chrismwendt
left a comment
chrismwendt
left a comment
There was a problem hiding this comment.
I mostly looked at the code_intelligence files and skimmed over the GitLab-specific code, LGTM
src/libs/github/util.tsx
Outdated
| } | ||
| } | ||
| } | ||
| console.log(baseCommitID, headCommitID) |
src/libs/gitlab/api.ts
Outdated
| * Response from the GitLab API for fetching a merge request. Note that there | ||
| * is more information returned but we are not using it. | ||
| */ | ||
| interface MRResponse { |
There was a problem hiding this comment.
I'd spell this out MergeRequestResponse
There was a problem hiding this comment.
That's how I originally had it and thought ResuestResponse sounded weird. This comment was enough to convince me to ignore that and spell it out
|
🎉 This PR is included in version 1.16.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
This PR brings support for GitLab.
Testing plan:
Run the browser extension and visit the following and check to ensure it works
Code intelligence
Search
Action: Enable search feature flag and open this page.
Expected result: A new tab should open on the sourcegraph search results page with the
repo:filterand search query in the search barI have tested on: