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.

Sign up for GitHub

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

Jump to bottom

Use relative links for commits, mentions, and issues in markdown #29427

Merged
merged 11 commits into from Mar 13, 2024
Merged

Use relative links for commits, mentions, and issues in markdown #29427

merged 11 commits into from Mar 13, 2024

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented Feb 26, 2024

Fixes #29404

Use relative links for

  • commits
  • mentions
  • issues

@KN4CK3R KN4CK3R added the type/enhancement An improvement of existing functionality label Feb 26, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 26, 2024
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 26, 2024
@lunny
Copy link
Member

lunny commented Feb 26, 2024

No, except you can know all the renderers will not be used on mail templates.

@delvh delvh changed the title Use relative links Use relative links for commits, mentions, and issues in markdown Feb 26, 2024
delvh
delvh approved these changes Feb 26, 2024
View reviewed changes
Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

The markdown renderer is only run for the UI and not for the API, right?

But yeah, I completely misunderstood the original request, I thought it talked about these headers (Reviewed-on, …) in commit messages.

@GiteaBot GiteaBot removed the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 26, 2024
@pull-request-size pull-request-size bot removed the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 26, 2024
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Feb 26, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 26, 2024
@KN4CK3R
Copy link
Member Author

KN4CK3R commented Feb 26, 2024

@lunny is right, mails would be broken. Fixed that and added a small test to prevent it in future.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 26, 2024
@KN4CK3R
Copy link
Member Author

KN4CK3R commented Feb 27, 2024

Don't know why the unit tests don't run in the workflow but running them locally revealed some errors in the tests. Reason is this:

gitea/modules/markup/markdown/markdown_test.go

Lines 24 to 26 in e55926e

AppURL = "http://localhost:3000/"
Repo = "gogits/gogs"
AppSubURL = AppURL + Repo + "/"

That's not how our setting.AppURL and setting.AppSubURL work. Will fix this later.

@KN4CK3R KN4CK3R marked this pull request as draft February 27, 2024 07:08
@lunny
Copy link
Member

lunny commented Feb 27, 2024

I encountered a similar problem in #29222, the test data are inconsistent.

@KN4CK3R KN4CK3R mentioned this pull request Feb 27, 2024
Merged
@KN4CK3R
Copy link
Member Author

KN4CK3R commented Feb 27, 2024

Added #29459 to fix the tests.

lunny pushed a commit that referenced this pull request Feb 29, 2024
@KN4CK3R
Fix wrong test usage of AppSubURL (#29459)
a6fd017 
The tests use an invalid `setting.AppSubURL`. The wrong behaviour
disturbs other PRs like #29222 and #29427.
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Feb 29, 2024
@KN4CK3R @GiteaBot
Fix wrong test usage of AppSubURL (go-gitea#29459)
90549e3 
The tests use an invalid `setting.AppSubURL`. The wrong behaviour
disturbs other PRs like go-gitea#29222 and go-gitea#29427.
@GiteaBot GiteaBot mentioned this pull request Feb 29, 2024
Merged
lunny pushed a commit that referenced this pull request Feb 29, 2024
@GiteaBot @KN4CK3R
Fix wrong test usage of AppSubURL (#29459) (#29488)
c293e34 
Backport #29459 by @KN4CK3R

The tests use an invalid `setting.AppSubURL`. The wrong behaviour
disturbs other PRs like #29222 and #29427.

Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
@wxiaoguang wxiaoguang added this to the 1.22.0 milestone Mar 6, 2024
@silverwind
Copy link
Member

Still WIP?

DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Mar 8, 2024
@KN4CK3R @Gusted
Port "Fix wrong test usage of AppSubURL"
8f55cf1 
- Port of go-gitea/gitea#29459
- The tests use an invalid `setting.AppSubURL`. The wrong behaviour
disturbs other PRs like go-gitea/gitea#29222
and go-gitea/gitea#29427.
@KN4CK3R KN4CK3R marked this pull request as ready for review March 8, 2024 16:09
KN4CK3R
KN4CK3R commented Mar 8, 2024
View reviewed changes
services/mailer/mail.go
@@ -222,7 +222,8 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient
body, err := markdown.RenderString(&markup.RenderContext{
Ctx: ctx,
Links: markup.Links{
Base: ctx.Issue.Repo.HTMLURL(),
AbsolutePrefix: true,
Copy link
Member Author

Choose a reason for hiding this comment

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

The mail, maybe /routers/common/markup.go and/or rendered webhook messages should be the only places with AbsolutePrefix = true. Some tests have true too but that should be changed in another PR.

@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 13, 2024
@silverwind silverwind enabled auto-merge (squash) March 13, 2024 09:54
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Mar 13, 2024
@silverwind silverwind merged commit 85c59d6 into go-gitea:main Mar 13, 2024
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 13, 2024
silverwind added a commit to silverwind/gitea that referenced this pull request Mar 13, 2024
@silverwind
Merge branch 'main' into overflow-menu
b1e4857 
* main:
  fix missed RenderLabel change in card template (go-gitea#29772)
  Fix incorrect locale Tr for gpg command (go-gitea#29754)
  Improve a11y document and dropdown item (go-gitea#29753)
  Improve QueryEscape helper function (go-gitea#29768)
  Use relative links for commits, mentions, and issues in markdown (go-gitea#29427)
  Move fork router functions to a standalone file (go-gitea#29756)
  Configure pinned JS dependencies via updates.config.js (go-gitea#29696)
  Refactor to use optional.Option for issue index search option (go-gitea#29739)
  Fix user router possbile panic (go-gitea#29751)
  Refactor label.IsArchived() (go-gitea#29750)
  Fix date rendering by adding `<gitea-absolute-date>` (go-gitea#29725)
  Update to labeler v5 (go-gitea#29721)
  Update Chroma to v2.13.0 (go-gitea#29732)
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 14, 2024
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
6e7fc2d 
* giteaofficial/main:
  Fix `make generate-swagger` in go 1.22 (go-gitea#29780)
  Fix incorrect menu/link on webhook edit page (go-gitea#29709)
  Add test for webhook (go-gitea#29755)
  Fix possible NPE in ToPullReviewList (go-gitea#29759)
  fix missed RenderLabel change in card template (go-gitea#29772)
  Fix incorrect locale Tr for gpg command (go-gitea#29754)
  Improve a11y document and dropdown item (go-gitea#29753)
  Improve QueryEscape helper function (go-gitea#29768)
  Use relative links for commits, mentions, and issues in markdown (go-gitea#29427)
  Move fork router functions to a standalone file (go-gitea#29756)
  Configure pinned JS dependencies via updates.config.js (go-gitea#29696)
  Refactor to use optional.Option for issue index search option (go-gitea#29739)
  Fix user router possbile panic (go-gitea#29751)
  Refactor label.IsArchived() (go-gitea#29750)
@KN4CK3R KN4CK3R deleted the enhancement-relative-link branch April 21, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lunny lunny lunny approved these changes

@silverwind silverwind silverwind approved these changes

@milahu milahu milahu left review comments

@delvh delvh delvh approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.

markdown: wrong hostname in commit url with empty ROOT_URL
7 participants
@KN4CK3R @lunny @silverwind @milahu @delvh @wxiaoguang @GiteaBot