Skip to content

Render inline code/file references #29133

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

Closed
wants to merge 15 commits into from

Conversation

Mai-Lapyst
Copy link
Contributor

This PR adds a preview box like github does for permalinks to a range of lines for a file in the repo, e.g: http://localhost:3000/mai-lapyst/testprojet_ci/src/commit/49fb6d29fad1933818f06eef1bf95f6d7235f3f6/some/deep/folder/test.js#L7-L19.

Closes #20359.

This works in both issues/PRs, as well as other markup rendering for example markdown files.
It's implemented as a processor in modules/markup/html.go.

It also supports the unicode warnings the "normal" code view of files inside the repo support, as shown in the example rendering.

Example renderings:
image

image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 11, 2024
@silverwind silverwind added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Feb 11, 2024
@Mai-Lapyst Mai-Lapyst force-pushed the markup-add-filepreview branch from c2c2752 to 23853f9 Compare February 15, 2024 16:18
@6543 6543 added this to the 1.22.0 milestone Feb 16, 2024
@silverwind
Copy link
Member

image

Does this yellow triangle render the same in normal code view? If not, I think it may be using some overly-specific CSS selector that fixes it only for the normal code view.

@Mai-Lapyst Mai-Lapyst force-pushed the markup-add-filepreview branch from aa32138 to 06fbaed Compare February 26, 2024 16:48
@Mai-Lapyst
Copy link
Contributor Author

image

Does this yellow triangle render the same in normal code view? If not, I think it may be using some overly-specific CSS selector that fixes it only for the normal code view.

It seems the exact same for me in testing. Could'nt find a specific css on the normal code view, even after rebasing onto main.

@lunny
Copy link
Member

lunny commented Mar 2, 2024

Please add some tests for newly introduced regex and the function.

@lunny
Copy link
Member

lunny commented Mar 2, 2024

Maybe it can also support branches and tags files but not commits only.

@lunny lunny modified the milestones: 1.22.0, 1.23.0 Mar 4, 2024
@Mai-Lapyst
Copy link
Contributor Author

Please add some tests for newly introduced regex and the function.

Have added the requested test.

Maybe it can also support branches and tags files but not commits only.

It would advise against it. These file previews are commonly used with permalinks to discuss a certain state in the codebase and thus need to refer to the state of those files in the moment the issue takes place. Using "dynamic" links, such as a branch or tag, where a commit isnt fixed, can lead to confusion since later the file contains not that what was talked about.

@silverwind
Copy link
Member

silverwind commented Mar 7, 2024

Needs a merge in services/markup/processorhelper.go. After that is fixed, I will test it.

@Mai-Lapyst Mai-Lapyst force-pushed the markup-add-filepreview branch from 5232975 to 69a6aa0 Compare March 8, 2024 00:31
@Mai-Lapyst
Copy link
Contributor Author

Needs a merge in services/markup/processorhelper.go. After that is fixed, I will test it.

@silverwind Have resolved the conflict and rebased onto main. You're welcome to test now :)

@Mai-Lapyst Mai-Lapyst closed this Mar 15, 2024
@Mai-Lapyst Mai-Lapyst deleted the markup-add-filepreview branch March 15, 2024 08:37
@lunny
Copy link
Member

lunny commented Mar 15, 2024

@Mai-Lapyst Why close this PR?

@GiteaBot GiteaBot removed this from the 1.23.0 milestone Mar 15, 2024
@silverwind
Copy link
Member

Sorry I wasn't able to review sooner. I think this was pretty close to mergeable.

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jun 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inline code references
6 participants