-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Only turn links to current instance into hash links #36237
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
base: main
Are you sure you want to change the base?
Conversation
| // only turn commit links to the current instance into hash link | ||
| if !strings.HasPrefix(strings.ToLower(ret.FullURL), strings.ToLower(setting.AppURL)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multidomain support #8697
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what do we check instead of setting.AppURL? Is there a slice of "current app urls" somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no "current app urls" at the moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could make server.ROOT_URL accept multiple URLs comma-separated, and then check against all of those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or is there a way to obtain the current URL/Origin in this function? Do we need to pass *http.Request or some context down all the way to this function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the only way to make it work based on request URL is to determine the current HTTP request origin (from HTTP headers Host, X-Forwarded-Host, X-Forwarded-Protocol and more) into markup.RenderContext, which will be quite complicated.
It'll certainly be easier to let the user configure multiple server.ROOT_URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this can be used:
Line 58 in eddf875
| func GuessCurrentAppURL(ctx context.Context) string { |
Given the following markdown:
Previously, all links would turn into hash link, even ones to external sites:
After this change, only links to the current instance, as identified by
setting.AppURLare turned into hash links:There is still one notable difference with GitHub where the second link should render like
user/repo@<hash>, not<hash>as currently, I would like to fix that here as well.