Skip to content

Use configurable remote name in snippets #35172

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
Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Use configurable remote name in snippets #35172

wants to merge 6 commits into from

Conversation

ilya-nurullin
Copy link

@ilya-nurullin ilya-nurullin commented Jul 28, 2025 •
edited
Loading

Closes #19403, and makes it possible to use any remote name in code snippets for an empty repository and pull request.
This change is very helpful to me, because I always use different name for my gitea remote.

Uses setting config module to store the value. Default is origin for backward compatibility.

Screenshots

Empty repo image
Pull Request image
Settings page image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 28, 2025
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/templates This PR modifies the template files labels Jul 28, 2025
@lunny
Copy link
Member

lunny commented Jul 28, 2025

Maybe it should be a per repository setting but not a global one.

@ilya-nurullin
Copy link
Author

Maybe it should be a per repository setting but not a global one.

I think we can get started with a global one, and then if needed, make it more precise.
At least, the global one works well for me, and I don't need more granular settings here.

@lunny
Copy link
Member

lunny commented Jul 28, 2025

Even if you intend to add a global configuration, please store it in the database rather than in app.ini.

@ilya-nurullin
Copy link
Author

ilya-nurullin commented Jul 29, 2025 •
edited
Loading

Even if you intend to add a global configuration, please store it in the database rather than in app.ini.

Done!

@silverwind silverwind added the docs-update-needed The document needs to be updated synchronously label Jul 30, 2025
lunny
lunny reviewed Jul 30, 2025
View reviewed changes
@lunny lunny added this to the 1.25.0 milestone Jul 30, 2025
@lunny
Copy link
Member

lunny commented Aug 1, 2025 •
edited
Loading

Does the title “Repository Snippets” in the configuration accurately reflect its purpose? It might be more appropriate under the Pull Request Settings section.

@ilya-nurullin
Copy link
Author

Does the title “Repository Snippets” in the configuration accurately reflect its purpose? It might be more appropriate under the Pull Request Settings section.

This setting affects two pages at once:

  1. Empty repo page - I believe this is the most frequent use case for the setting
  2. Pull Request page - you can do a merge using the button on the page, so snippets are hidden by default

Therefore, I would choose a more general name. I am open to any suggestions.

@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 Aug 3, 2025
@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 Aug 4, 2025
@lunny lunny added reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. and removed docs-update-needed The document needs to be updated synchronously labels Aug 4, 2025
wxiaoguang
wxiaoguang reviewed Aug 4, 2025
View reviewed changes
services/context/repo.go
@@ -542,6 +542,7 @@ func RepoAssignment(ctx *Context) {
ctx.Data["CanWriteIssues"] = ctx.Repo.CanWrite(unit_model.TypeIssues)
ctx.Data["CanWritePulls"] = ctx.Repo.CanWrite(unit_model.TypePullRequests)
ctx.Data["CanWriteActions"] = ctx.Repo.CanWrite(unit_model.TypeActions)
ctx.Data["SnippetRemoteName"] = setting.Config().Repository.SnippetRemoteName.Value(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and above: we can remove this SnippetRemoteName from template data, and get its value directly from the templates that need it.

Copy link
Author

Choose a reason for hiding this comment

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

Since it is used in several places in the template, I think it makes sense to extract it into a variable, which is what I did.

wxiaoguang
wxiaoguang reviewed Aug 4, 2025
View reviewed changes
templates/admin/config_settings.tmpl
<form class="ui form form-fetch-action" method="post" action="{{AppSubUrl}}/-/admin/config?key={{.SystemConfig.Repository.SnippetRemoteName.DynKey}}">
<div class="field" data-field-patched="true">
<label for="remote_name">{{ctx.Locale.Tr "admin.config.repository_snippets.remote_name"}}</label>
<input id="remote_name" name="value" value="{{.SystemConfig.Repository.SnippetRemoteName.Value ctx}}" maxlength="100" dir="auto" placeholder="{{.SystemConfig.Repository.SnippetRemoteName.Def}}">
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the Def here is useful.

The value is always filled by Value when a user opens this page.

So I think we should remove the Def function.

Copy link
Author

Choose a reason for hiding this comment

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

The main reason for Def is being able to get the default value when user removes value on settings page.
It is used here: https://github.com/go-gitea/gitea/pull/35172/files#diff-b3b66aef8cc83304cd5cfc661233e6c1c1be656fbd6cd1e09b114348f224ff32R251

In current architecture func (value *Value[T]) Value(ctx context.Context) (v T) function uses a default value only if it is not present in the database. So null in db, nil in go values are valid ones, and therefore the default value won't be used, and as a result there won't be any remote name on the page in snippets.

wxiaoguang
wxiaoguang reviewed Aug 4, 2025
View reviewed changes
modules/setting/config.go
@@ -49,6 +49,7 @@ func DefaultOpenWithEditorApps() OpenWithEditorAppsType {

type RepositoryStruct struct {
OpenWithEditorApps *config.Value[OpenWithEditorAppsType]
SnippetRemoteName *config.Value[string]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Snippet a proper name?

IMO it is not a "snippet", it is for "git guide for users"

Copy link
Author

@ilya-nurullin ilya-nurullin Aug 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

What name would you use here? Can it be just RemoteName?

@wxiaoguang wxiaoguang removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 4, 2025
@wxiaoguang wxiaoguang marked this pull request as draft August 4, 2025 03:33
wxiaoguang
wxiaoguang reviewed Aug 4, 2025
View reviewed changes
options/locale/locale_en-US.ini
@@ -3419,6 +3419,9 @@ config.disable_gravatar = Disable Gravatar
config.enable_federated_avatar = Enable Federated Avatars
config.open_with_editor_app_help = The "Open with" editors for the clone menu. If left empty, the default will be used. Expand to see the default.

config.repository_snippets = Repository Snippets
Copy link
Contributor

Choose a reason for hiding this comment

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

It's also an unclear name ....

Repository Snippets reads like "some parts of the repository", and GitLab users have been familiar with its Code Snippets https://docs.gitlab.com/user/snippets/

Copy link
Author

Choose a reason for hiding this comment

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

Can you suggest a better name?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

@lunny lunny lunny approved these changes

@hiifong hiifong hiifong 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 modifies/templates This PR modifies the template files modifies/translation
Projects
None yet
Milestone
1.25.0
Development

Successfully merging this pull request may close these issues.

Change default remote name
6 participants
@ilya-nurullin @lunny @wxiaoguang @hiifong @silverwind @GiteaBot