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 "edit file" and "browse repo" links #883

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dschiemann80
Copy link

Hi zoekt-Team,

I am using zoekt with a neogrok frontend. I would like to be able to display more links in the neogrok search results. I extended the zoekt SearchResult with two new maps RepoEditURLs and RepoBrowseURLs.
RepoEditURLs is a map repo => template for generating deep-links directly to Edit pages of source code in web interfaces (I added as many templates to gitindex/index.go as I could).
RepoBrowseURLs is a map => string for links leading to the repository the file match in the search result is contained in.
Please review and tell me if you think this is useful.

Regards,
Daniel

extend SearchResult with two new maps RepoEditURLs and RepoBrowseURLs
Copy link

cla-bot bot commented Jan 7, 2025

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Daniel Schiemann.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@dschiemann80
Copy link
Author

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Daniel Schiemann. This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits `git config --list | grep email`

2. If not, set it up using `git config --global user.email [email protected]`

3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

I added my email address to my GitHub account, but I can't find a way to re-run the failed check.

@jtibshirani
Copy link
Member

@cla-bot check

Copy link

cla-bot bot commented Jan 8, 2025

We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added.

Sourcegraph teammates:

Copy link

cla-bot bot commented Jan 8, 2025

The GitHub CLA Bot is rechecking to see that you have signed the CLA - note that it may take up to 30 minutes for your response to be synchronized.

For Sourcegraph teammates: see how-to diagnose CLA Bot failures.

@burmudar
Copy link
Contributor

burmudar commented Jan 9, 2025

Hey @dschiemann80 could you try signing the CLA again? Seems your information didn't get persisted 🙏🏼

@dschiemann80
Copy link
Author

Hey @dschiemann80 could you try signing the CLA again? Seems your information didn't get persisted 🙏🏼

I signed the CLA now, first had to clarify with my employer.

@dschiemann80 dschiemann80 marked this pull request as draft January 9, 2025 10:02
@dschiemann80
Copy link
Author

I converted this PR into a draft, because I have a few questions after researching yesterday and I am not sure if the PR in this form is the way to go forward.
I was wondering why the custom URLJoinPath() templating function used in some templates is not rendered in neogrok. I found out, that this is by design. URLJoinPath() gets rendered for the HTML interface exposed by zoekt-webserver itself, but the template doesn't get rendered at all for the GRPC case. This makes sense, because in the GRPC case, the receiving frontend (in my case neogrok), does the rendering with simple string replacement. But neogrok (or any other frontend using GRPC) doesn't know the URLPathJoin() function. I am not sure how to continue from here.
Should a similar function get implemented in every frontend using zoekt GRPC?
Why use a custom templating function in the first place and not just something like url.QueryEscape(s) from net/url?
And why is special case handling needed for the "+"? The browsers I tested were able to open URLs with a "+" just fine, not even escaping the "+" (maybe they just hide this).
Please advise how to continue.

Copy link
Member

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dschiemann80 thanks for your interest in Zoekt!

SearchResult is a core struct in the API, which we try to keep quite streamlined. Instead of storing this information in Zoekt, would it be possible to create the repo browse and edit URLs in your application? Maybe you could transform the RepoURLs, which are already available on the search result. Or these really need to be in Zoekt for some reason?

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.

3 participants