Fix various trivial problems (#32861)

1. add/improve comments to help future readers could understand the
problem more easily.
2. add an error log to LDAP with username fallback
3. use `or` instead of `Iif` for "repo/branch_dropdown" (`Iif` was a
mistake, but it doesn't really affect the UI)
4. add `tw-font-mono` style to container digest to match dockerhub
5. fix a bug in RepoBranchTagSelector: the form is not updated when
there is no click to an item

---------

Co-authored-by: delvh <[email protected]>

Author: wxiaoguang

models/actions/run_job_status_test.go

@@ -69,7 +69,7 @@ func TestAggregateJobStatus(t *testing.T) {
 		{[]Status{StatusFailure, StatusBlocked}, StatusFailure},
 
 		// skipped with other status
-		// TODO: need to clarify whether a PR with "skipped" job status is considered as "mergeable" or not.
+		// "all skipped" is also considered as "mergeable" by "services/actions.toCommitStatus", the same as GitHub
 		{[]Status{StatusSkipped}, StatusSkipped},
 		{[]Status{StatusSkipped, StatusSuccess}, StatusSuccess},
 		{[]Status{StatusSkipped, StatusFailure}, StatusFailure},

modules/markup/markdown/markdown.go

@@ -129,7 +129,8 @@ func SpecializedMarkdown(ctx *markup.RenderContext) *GlodmarkRender {
 				Enabled:           setting.Markdown.EnableMath,
 				ParseDollarInline: true,
 				ParseDollarBlock:  true,
-				ParseSquareBlock:  true, // TODO: this is a bad syntax, it should be deprecated in the future (by some config options)
+				ParseSquareBlock:  true, // TODO: this is a bad syntax "\[ ... \]", it conflicts with normal markdown escaping, it should be deprecated in the future (by some config options)
+				// ParseBracketInline: true, // TODO: this is also a bad syntax "\( ... \)", it also conflicts, it should be deprecated in the future
 			}),
 			meta.Meta,
 		),

modules/repository/fork.go

@@ -12,6 +12,9 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 )
 
+// CanUserForkBetweenOwners returns true if user can fork between owners.
+// By default, a user can fork a repository from another owner, but not from themselves.
+// Many users really like to fork their own repositories, so add an experimental setting to allow this.
 func CanUserForkBetweenOwners(id1, id2 int64) bool {
 	if id1 != id2 {
 		return true

routers/web/repo/actions/view.go

@@ -205,8 +205,11 @@ func ViewPost(ctx *context_module.Context) {
 		}
 	}
 
+	// TODO: "ComposeMetas" (usually for comment) is not quite right, but it is still the same as what template "RenderCommitMessage" does.
+	// need to be refactored together in the future
 	metas := ctx.Repo.Repository.ComposeMetas(ctx)
 
+	// the title for the "run" is from the commit message
 	resp.State.Run.Title = run.Title
 	resp.State.Run.TitleHTML = templates.NewRenderUtils(ctx).RenderCommitMessage(run.Title, metas)
 	resp.State.Run.Link = run.Link()

services/auth/source/ldap/source_authenticate.go

@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/models/auth"
 	user_model "code.gitea.io/gitea/models/user"
 	auth_module "code.gitea.io/gitea/modules/auth"
+	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/optional"
 	asymkey_service "code.gitea.io/gitea/services/asymkey"
 	source_service "code.gitea.io/gitea/services/auth/source"
@@ -31,7 +32,12 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u
 		return nil, user_model.ErrUserNotExist{Name: loginName}
 	}
 	// Fallback.
+	// FIXME: this fallback would cause problems when the "Username" attribute is not set and a user inputs their email.
+	// In this case, the email would be used as the username, and will cause the "CreateUser" failure for the first login.
 	if sr.Username == "" {
+		if strings.Contains(userName, "@") {
+			log.Error("No username in search result (Username Attribute is not set properly?), using email as username might cause problems")
+		}
 		sr.Username = userName
 	}
 	if sr.Mail == "" {

templates/package/content/container.tmpl

@@ -36,9 +36,10 @@
 				</thead>
 				<tbody>
 					{{range .PackageDescriptor.Metadata.Manifests}}
+						{{/* "unknown/unknown" is attestation-manifest, so we should skip it */}}
 						{{if ne .Platform "unknown/unknown"}}
 						<tr>
-							<td><a href="{{$.PackageDescriptor.PackageWebLink}}/{{PathEscape .Digest}}">{{StringUtils.TrimPrefix .Digest "sha256:" | ShortSha}}</a></td>
+							<td><a class="tw-font-mono" href="{{$.PackageDescriptor.PackageWebLink}}/{{PathEscape .Digest}}">{{StringUtils.TrimPrefix .Digest "sha256:" | ShortSha}}</a></td>
 							<td>{{.Platform}}</td>
 							<td>{{FileSize .Size}}</td>
 						</tr>

templates/repo/commit_page.tmpl

@@ -68,11 +68,13 @@
 											<p id="cherry-pick-content" class="branch-dropdown"></p>
 
 											<form method="get">
+												{{/*FIXME: CurrentRefShortName seems not making sense here (old code),
+												because the "commit page" has no "$.BranchName" info, so only using DefaultBranch should be enough */}}
 												{{template "repo/branch_dropdown" dict
 													"Repository" .Repository
 													"ShowTabBranches" true
 													"CurrentRefType" "branch"
-													"CurrentRefShortName" (Iif $.BranchName $.Repository.DefaultBranch)
+													"CurrentRefShortName" (or $.BranchName $.Repository.DefaultBranch)
 													"RefFormActionTemplate" (print "{RepoLink}/_cherrypick/" .CommitID "/{RefShortName}")
 												}}
 												<input type="hidden" id="cherry-pick-type" name="cherry-pick-type"><br>

templates/repo/issue/filter_item_label.tmpl

@@ -23,6 +23,9 @@
 		<div class="divider"></div>
 		<a class="item label-filter-query-default" href="{{QueryBuild $queryLink "labels" NIL}}">{{ctx.Locale.Tr "repo.issues.filter_label_no_select"}}</a>
 		<a class="item label-filter-query-not-set" href="{{QueryBuild $queryLink "labels" 0}}">{{ctx.Locale.Tr "repo.issues.filter_label_select_no_label"}}</a>
+		{{/* The logic here is not the same as the label selector in the issue sidebar.
+		The one in the issue sidebar renders "repo labels | divider | org labels".
+		Maybe the logic should be updated to be consistent.*/}}
 		{{$previousExclusiveScope := "_no_scope"}}
 		{{range .Labels}}
 			{{$exclusiveScope := .ExclusiveScope}}

templates/user/auth/signin_inner.tmpl

@@ -48,6 +48,7 @@
 			</div>
 		</form>
 		{{end}}{{/*if .EnablePasswordSignInForm*/}}
+		{{/* "oauth_container" contains not only "oauth2" methods, but also "OIDC" and "SSPI" methods */}}
 		{{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}}
 		{{if and $showOAuth2Methods .EnablePasswordSignInForm}}
 			<div class="divider divider-text">{{ctx.Locale.Tr "sign_in_or"}}</div>

templates/user/auth/signup_inner.tmpl

@@ -47,6 +47,8 @@
 					</button>
 				</div>
 			{{end}}
+			{{/* "oauth_container" contains not only "oauth2" methods, but also "OIDC" and "SSPI" methods */}}
+			{{/* TODO: it seems that "EnableSSPI" is only set in "sign-in" handlers, but it should use the same logic to control its display */}}
 			{{$showOAuth2Methods := or .OAuth2Providers .EnableOpenIDSignIn .EnableSSPI}}
 			{{if $showOAuth2Methods}}
 				<div class="divider divider-text">{{ctx.Locale.Tr "sign_in_or"}}</div>