Skip to content

add submodule diff links #33097

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
Merged
merged 17 commits into from
Jan 8, 2025
Merged

add submodule diff links #33097

merged 17 commits into from
Jan 8, 2025

Conversation

bohde
Copy link
Contributor

@bohde bohde commented Jan 3, 2025

This adds links to submodules in diffs, similar to the existing link when viewing a repo at a specific commit. It does this by expanding diff parsing to recognize changes to submodules, and find the specific refs that are added, deleted or changed.

The templates are updated to add either a link to the submodule at a commit, or the diff between two commits in the event that the submodule is updated.

A slight refactor was done to simplify calling RefURL on the submodule. There was also a FIXME comment in the template that said this should be updated to account for setting.AppSubURL. I tested this in an environment that uses a non default setting.AppSubURL, and verified that it works.

Related #25888

@bohde
add submodule diff links
b218c65 
This adds links to submodules in diffs, similar to the existing link
when viewing a repo at a specific commit. It does this by expanding
diff parsing to recognize changes to submodules, and find the specific
refs that are added, deleted or changed.

The templates are updated to add either a link to the submodule at a
commit, or the diff between two commits in the event that the
submodule is updated.

A slight refactor was done to simplify calling RefURL on the
submodule.
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 3, 2025
@github-actions github-actions bot added modifies/translation modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels Jan 3, 2025
@bohde bohde marked this pull request as ready for review January 3, 2025 20:20
@lunny lunny added this to the 1.24.0 milestone Jan 4, 2025
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jan 4, 2025 •
edited
Loading

ps: we need this one first: Make git clone URL could use current signed-in user #33091 : it make the Git URL parsing more general (parseRepositoryURL) and need to be used here to replace the getRefURL

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jan 4, 2025 •
edited
Loading

Made some changes in 8d63859:

  1. avoid using HTML trick in translations
  2. hide diff-header-popup-btn for submodules, otherwise there is an empty popup-menu
  3. remove ui message from submodule's block, otherwise there are double-borders
  4. use {{- -}} to remove extra spaces, otherwise the generated HTML page contains too many unnecessary spaces

ps: need this one "Make git clone URL could use current signed-in user #33091" to rewrite RefURL.

image

bohde
bohde commented Jan 6, 2025
View reviewed changes
bohde
bohde commented Jan 6, 2025
View reviewed changes
@wxiaoguang
Copy link
Contributor

By the way, Make git clone URL could use current signed-in user #33091 still needs one more review. It blocks this one because we need to use unified the parseRepositoryURL.

@pull-request-size pull-request-size bot added size/XL and removed size/L labels Jan 7, 2025
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jan 7, 2025 •
edited
Loading

About the translation: #33097 (comment)

Another choice is something like: Submodule %[1]s updated: 11111...22222, what do you think?

image

The new changes after 68a8488:

  1. use the unified function to parse submodule URL, now we use relative links for our repo and full link for others (eg: GitHub), the old function getRefURL and its tests are all removed
  2. rename SubmoduleInfo to SubmoduleDiffInfo, since it is only for the "diff"
  3. remove RefID, construct links in Go code, now the template code is simpler than before.

wxiaoguang
wxiaoguang approved these changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

CI passes and it's almost done on my side.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 7, 2025
@bohde
Copy link
Contributor Author

bohde commented Jan 7, 2025

I can't approve since it's my PR, but @wxiaoguang's changes look good to me!

@lunny
Copy link
Member

lunny commented Jan 7, 2025

Can we display a different file type icon for sub module in the left tree in this PR?

图片

@bohde
Copy link
Contributor Author

bohde commented Jan 7, 2025

Can we display a different file type icon for sub module in the left tree in this PR?

图片

This should be fixed in ee74ae0

Screenshot 2025-01-07 at 3 21 44 PM

@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 Jan 7, 2025
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jan 7, 2025
@wxiaoguang wxiaoguang enabled auto-merge (squash) January 8, 2025 00:44
@wxiaoguang wxiaoguang merged commit a8e7cae into go-gitea:main Jan 8, 2025
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 Jan 8, 2025
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 8, 2025
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
b2c0630 
* giteaofficial/main:
  add submodule diff links (go-gitea#33097)
  [skip ci] Updated translations via Crowdin
  Update status check for all supported on.pull_request.types in Gitea (go-gitea#33117)
  Support the new exit code for `git remote` subcommands for git version >=2.30.0 (go-gitea#33129)
  Make git clone URL could use current signed-in user (go-gitea#33091)
  Filter reviews of one pull request in memory instead of database to reduce slow response because of lacking database index (go-gitea#33106)
This was referenced Jan 8, 2025
Closed
Closed
@bohde bohde mentioned this pull request Jan 28, 2025
Merged
@lunny lunny mentioned this pull request Jan 29, 2025
Closed
lunny pushed a commit that referenced this pull request Jan 30, 2025
@bohde
Link to tree views of submodules if possible (#33424)
ac2d97c 
This is a follow-up to #33097. 

When linking a submodule at a commit in either the repo view, or a diff
when adding a new submodule, link to the tree view of that submodules
intead of the individual commit. This shows the user the full tree,
instead of the diff of the commit.

This makes the assumption that the tree for a given SHA is at
`<repo_url>/tree/<sha>`. This URL format is supported by both Github &
Gitlab, but not Gitea. To fix this, add a redirect from
`<username>/<repo>/tree/<ref>` to `<username>/<repo>/src/<ref>`, so that
Gitea can support this URL structure.
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Apr 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@wxiaoguang wxiaoguang wxiaoguang 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/dependencies modifies/frontend modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files modifies/translation
Projects
None yet
Milestone
1.24.0
Development

Successfully merging this pull request may close these issues.
4 participants
@bohde @wxiaoguang @lunny @GiteaBot