From ff14ada9655660af15d615b2fbd090b861686cd2 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Fri, 6 Dec 2024 00:11:35 +0200 Subject: [PATCH 1/3] Bump relative-time-element to v4.4.4 (#32730) Fix #32716 Tested, it still works. - cc @wxiaoguang for https://github.com/github/relative-time-element/pull/296 Signed-off-by: Yarden Shoham --- package-lock.json | 8 ++++---- package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package-lock.json b/package-lock.json index 2261566c02..e3f7a0116f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "@citation-js/plugin-csl": "0.7.14", "@citation-js/plugin-software-formats": "0.6.1", "@github/markdown-toolbar-element": "2.2.3", - "@github/relative-time-element": "4.4.3", + "@github/relative-time-element": "4.4.4", "@github/text-expander-element": "2.8.0", "@mcaptcha/vanilla-glue": "0.1.0-alpha-3", "@primer/octicons": "19.13.0", @@ -3025,9 +3025,9 @@ "license": "MIT" }, "node_modules/@github/relative-time-element": { - "version": "4.4.3", - "resolved": "https://registry.npmjs.org/@github/relative-time-element/-/relative-time-element-4.4.3.tgz", - "integrity": "sha512-EVKokqx9/DdUAZ2l9WVyY51EtRCO2gQWWMvsRIn7r4glJ91q9CXcnILVHZVCpfD52ucXUhUvtYsAjNJ4qP4uIg==", + "version": "4.4.4", + "resolved": "https://registry.npmjs.org/@github/relative-time-element/-/relative-time-element-4.4.4.tgz", + "integrity": "sha512-Oi8uOL8O+ZWLD7dHRWCkm2cudcTYtB3VyOYf9BtzCgDGm+OKomyOREtItNMtWl1dxvec62BTKErq36uy+RYxQg==", "license": "MIT" }, "node_modules/@github/text-expander-element": { diff --git a/package.json b/package.json index 095deb28fa..d30aedc54f 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "@citation-js/plugin-csl": "0.7.14", "@citation-js/plugin-software-formats": "0.6.1", "@github/markdown-toolbar-element": "2.2.3", - "@github/relative-time-element": "4.4.3", + "@github/relative-time-element": "4.4.4", "@github/text-expander-element": "2.8.0", "@mcaptcha/vanilla-glue": "0.1.0-alpha-3", "@primer/octicons": "19.13.0", From f7f68e4cc02ca57b841e20e0975b8926bf5c3722 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 6 Dec 2024 12:04:16 +0800 Subject: [PATCH 2/3] Refactor RepoActionView.vue, add `::group::` support (#32713) 1. make it able to "force reload", then the previous pending request won't block the new request 2. make it support `::group::` 3. add some TS types (but there are still many variables untyped, this PR is large enough, the remaining types could be added in the future) --- routers/web/devtest/mock_actions.go | 108 ++++++++++++ routers/web/repo/actions/view.go | 98 +++++------ routers/web/web.go | 10 +- templates/devtest/repo-action-view.tmpl | 30 ++++ web_src/js/components/RepoActionView.vue | 207 ++++++++++++----------- 5 files changed, 299 insertions(+), 154 deletions(-) create mode 100644 routers/web/devtest/mock_actions.go create mode 100644 templates/devtest/repo-action-view.tmpl diff --git a/routers/web/devtest/mock_actions.go b/routers/web/devtest/mock_actions.go new file mode 100644 index 0000000000..37e94aa802 --- /dev/null +++ b/routers/web/devtest/mock_actions.go @@ -0,0 +1,108 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package devtest + +import ( + "fmt" + mathRand "math/rand/v2" + "net/http" + "strings" + "time" + + actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/modules/web" + "code.gitea.io/gitea/routers/web/repo/actions" + "code.gitea.io/gitea/services/context" +) + +func generateMockStepsLog(logCur actions.LogCursor) (stepsLog []*actions.ViewStepLog) { + mockedLogs := []string{ + "::group::test group for: step={step}, cursor={cursor}", + "in group msg for: step={step}, cursor={cursor}", + "in group msg for: step={step}, cursor={cursor}", + "in group msg for: step={step}, cursor={cursor}", + "::endgroup::", + "message for: step={step}, cursor={cursor}", + "message for: step={step}, cursor={cursor}", + "message for: step={step}, cursor={cursor}", + "message for: step={step}, cursor={cursor}", + "message for: step={step}, cursor={cursor}", + } + cur := logCur.Cursor // usually the cursor is the "file offset", but here we abuse it as "line number" to make the mock easier, intentionally + for i := 0; i < util.Iif(logCur.Step == 0, 3, 1); i++ { + logStr := mockedLogs[int(cur)%len(mockedLogs)] + cur++ + logStr = strings.ReplaceAll(logStr, "{step}", fmt.Sprintf("%d", logCur.Step)) + logStr = strings.ReplaceAll(logStr, "{cursor}", fmt.Sprintf("%d", cur)) + stepsLog = append(stepsLog, &actions.ViewStepLog{ + Step: logCur.Step, + Cursor: cur, + Started: time.Now().Unix() - 1, + Lines: []*actions.ViewStepLogLine{ + {Index: cur, Message: logStr, Timestamp: float64(time.Now().UnixNano()) / float64(time.Second)}, + }, + }) + } + return stepsLog +} + +func MockActionsRunsJobs(ctx *context.Context) { + req := web.GetForm(ctx).(*actions.ViewRequest) + + resp := &actions.ViewResponse{} + resp.Artifacts = append(resp.Artifacts, &actions.ArtifactsViewItem{ + Name: "artifact-a", + Size: 100 * 1024, + Status: "expired", + }) + resp.Artifacts = append(resp.Artifacts, &actions.ArtifactsViewItem{ + Name: "artifact-b", + Size: 1024 * 1024, + Status: "completed", + }) + resp.State.CurrentJob.Steps = append(resp.State.CurrentJob.Steps, &actions.ViewJobStep{ + Summary: "step 0 (mock slow)", + Duration: time.Hour.String(), + Status: actions_model.StatusRunning.String(), + }) + resp.State.CurrentJob.Steps = append(resp.State.CurrentJob.Steps, &actions.ViewJobStep{ + Summary: "step 1 (mock fast)", + Duration: time.Hour.String(), + Status: actions_model.StatusRunning.String(), + }) + resp.State.CurrentJob.Steps = append(resp.State.CurrentJob.Steps, &actions.ViewJobStep{ + Summary: "step 2 (mock error)", + Duration: time.Hour.String(), + Status: actions_model.StatusRunning.String(), + }) + if len(req.LogCursors) == 0 { + ctx.JSON(http.StatusOK, resp) + return + } + + resp.Logs.StepsLog = []*actions.ViewStepLog{} + doSlowResponse := false + doErrorResponse := false + for _, logCur := range req.LogCursors { + if !logCur.Expanded { + continue + } + doSlowResponse = doSlowResponse || logCur.Step == 0 + doErrorResponse = doErrorResponse || logCur.Step == 2 + resp.Logs.StepsLog = append(resp.Logs.StepsLog, generateMockStepsLog(logCur)...) + } + if doErrorResponse { + if mathRand.Float64() > 0.5 { + ctx.Error(http.StatusInternalServerError, "devtest mock error response") + return + } + } + if doSlowResponse { + time.Sleep(time.Duration(3000) * time.Millisecond) + } else { + time.Sleep(time.Duration(100) * time.Millisecond) // actually, frontend reload every 1 second, any smaller delay is fine + } + ctx.JSON(http.StatusOK, resp) +} diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index f86d4c6177..0f0d7d1ebd 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -66,15 +66,25 @@ func View(ctx *context_module.Context) { ctx.HTML(http.StatusOK, tplViewActions) } +type LogCursor struct { + Step int `json:"step"` + Cursor int64 `json:"cursor"` + Expanded bool `json:"expanded"` +} + type ViewRequest struct { - LogCursors []struct { - Step int `json:"step"` - Cursor int64 `json:"cursor"` - Expanded bool `json:"expanded"` - } `json:"logCursors"` + LogCursors []LogCursor `json:"logCursors"` +} + +type ArtifactsViewItem struct { + Name string `json:"name"` + Size int64 `json:"size"` + Status string `json:"status"` } type ViewResponse struct { + Artifacts []*ArtifactsViewItem `json:"artifacts"` + State struct { Run struct { Link string `json:"link"` @@ -146,6 +156,25 @@ type ViewStepLogLine struct { Timestamp float64 `json:"timestamp"` } +func getActionsViewArtifacts(ctx context.Context, repoID, runIndex int64) (artifactsViewItems []*ArtifactsViewItem, err error) { + run, err := actions_model.GetRunByIndex(ctx, repoID, runIndex) + if err != nil { + return nil, err + } + artifacts, err := actions_model.ListUploadedArtifactsMeta(ctx, run.ID) + if err != nil { + return nil, err + } + for _, art := range artifacts { + artifactsViewItems = append(artifactsViewItems, &ArtifactsViewItem{ + Name: art.ArtifactName, + Size: art.FileSize, + Status: util.Iif(art.Status == actions_model.ArtifactStatusExpired, "expired", "completed"), + }) + } + return artifactsViewItems, nil +} + func ViewPost(ctx *context_module.Context) { req := web.GetForm(ctx).(*ViewRequest) runIndex := getRunIndex(ctx) @@ -157,11 +186,19 @@ func ViewPost(ctx *context_module.Context) { } run := current.Run if err := run.LoadAttributes(ctx); err != nil { - ctx.Error(http.StatusInternalServerError, err.Error()) + ctx.ServerError("run.LoadAttributes", err) return } + var err error resp := &ViewResponse{} + resp.Artifacts, err = getActionsViewArtifacts(ctx, ctx.Repo.Repository.ID, runIndex) + if err != nil { + if !errors.Is(err, util.ErrNotExist) { + ctx.ServerError("getActionsViewArtifacts", err) + return + } + } resp.State.Run.Title = run.Title resp.State.Run.Link = run.Link() @@ -205,12 +242,12 @@ func ViewPost(ctx *context_module.Context) { var err error task, err = actions_model.GetTaskByID(ctx, current.TaskID) if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error()) + ctx.ServerError("actions_model.GetTaskByID", err) return } task.Job = current if err := task.LoadAttributes(ctx); err != nil { - ctx.Error(http.StatusInternalServerError, err.Error()) + ctx.ServerError("task.LoadAttributes", err) return } } @@ -278,7 +315,7 @@ func ViewPost(ctx *context_module.Context) { offset := task.LogIndexes[index] logRows, err := actions.ReadLogs(ctx, task.LogInStorage, task.LogFilename, offset, length) if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error()) + ctx.ServerError("actions.ReadLogs", err) return } @@ -555,49 +592,6 @@ func getRunJobs(ctx *context_module.Context, runIndex, jobIndex int64) (*actions return jobs[0], jobs } -type ArtifactsViewResponse struct { - Artifacts []*ArtifactsViewItem `json:"artifacts"` -} - -type ArtifactsViewItem struct { - Name string `json:"name"` - Size int64 `json:"size"` - Status string `json:"status"` -} - -func ArtifactsView(ctx *context_module.Context) { - runIndex := getRunIndex(ctx) - run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex) - if err != nil { - if errors.Is(err, util.ErrNotExist) { - ctx.Error(http.StatusNotFound, err.Error()) - return - } - ctx.Error(http.StatusInternalServerError, err.Error()) - return - } - artifacts, err := actions_model.ListUploadedArtifactsMeta(ctx, run.ID) - if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error()) - return - } - artifactsResponse := ArtifactsViewResponse{ - Artifacts: make([]*ArtifactsViewItem, 0, len(artifacts)), - } - for _, art := range artifacts { - status := "completed" - if art.Status == actions_model.ArtifactStatusExpired { - status = "expired" - } - artifactsResponse.Artifacts = append(artifactsResponse.Artifacts, &ArtifactsViewItem{ - Name: art.ArtifactName, - Size: art.FileSize, - Status: status, - }) - } - ctx.JSON(http.StatusOK, artifactsResponse) -} - func ArtifactsDeleteView(ctx *context_module.Context) { if !ctx.Repo.CanWrite(unit.TypeActions) { ctx.Error(http.StatusForbidden, "no permission") diff --git a/routers/web/web.go b/routers/web/web.go index 6113a6457b..85e0fdc41e 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1424,7 +1424,6 @@ func registerRoutes(m *web.Router) { }) m.Post("/cancel", reqRepoActionsWriter, actions.Cancel) m.Post("/approve", reqRepoActionsWriter, actions.Approve) - m.Get("/artifacts", actions.ArtifactsView) m.Get("/artifacts/{artifact_name}", actions.ArtifactsDownloadView) m.Delete("/artifacts/{artifact_name}", actions.ArtifactsDeleteView) m.Post("/rerun", reqRepoActionsWriter, actions.Rerun) @@ -1626,9 +1625,12 @@ func registerRoutes(m *web.Router) { } if !setting.IsProd { - m.Any("/devtest", devtest.List) - m.Any("/devtest/fetch-action-test", devtest.FetchActionTest) - m.Any("/devtest/{sub}", devtest.Tmpl) + m.Group("/devtest", func() { + m.Any("", devtest.List) + m.Any("/fetch-action-test", devtest.FetchActionTest) + m.Any("/{sub}", devtest.Tmpl) + m.Post("/actions-mock/runs/{run}/jobs/{job}", web.Bind(actions.ViewRequest{}), devtest.MockActionsRunsJobs) + }) } m.NotFound(func(w http.ResponseWriter, req *http.Request) { diff --git a/templates/devtest/repo-action-view.tmpl b/templates/devtest/repo-action-view.tmpl new file mode 100644 index 0000000000..1fa71c0e5f --- /dev/null +++ b/templates/devtest/repo-action-view.tmpl @@ -0,0 +1,30 @@ +{{template "base/head" .}} +
+
+
+{{template "base/footer" .}} diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 7adc29ad41..909308262e 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -2,10 +2,22 @@ import {SvgIcon} from '../svg.ts'; import ActionRunStatus from './ActionRunStatus.vue'; import {createApp} from 'vue'; -import {toggleElem} from '../utils/dom.ts'; +import {createElementFromAttrs, toggleElem} from '../utils/dom.ts'; import {formatDatetime} from '../utils/time.ts'; import {renderAnsi} from '../render/ansi.ts'; -import {GET, POST, DELETE} from '../modules/fetch.ts'; +import {POST, DELETE} from '../modules/fetch.ts'; + +// see "models/actions/status.go", if it needs to be used somewhere else, move it to a shared file like "types/actions.ts" +type RunStatus = 'unknown' | 'waiting' | 'running' | 'success' | 'failure' | 'cancelled' | 'skipped' | 'blocked'; + +type LogLine = { + index: number; + timestamp: number; + message: string; +}; + +const LogLinePrefixGroup = '::group::'; +const LogLinePrefixEndGroup = '::endgroup::'; const sfc = { name: 'RepoActionView', @@ -23,7 +35,7 @@ const sfc = { data() { return { // internal state - loading: false, + loadingAbortController: null, intervalID: null, currentJobStepsStates: [], artifacts: [], @@ -89,9 +101,7 @@ const sfc = { // load job data and then auto-reload periodically // need to await first loadJob so this.currentJobStepsStates is initialized and can be used in hashChangeListener await this.loadJob(); - this.intervalID = setInterval(() => { - this.loadJob(); - }, 1000); + this.intervalID = setInterval(() => this.loadJob(), 1000); document.body.addEventListener('click', this.closeDropdown); this.hashChangeListener(); window.addEventListener('hashchange', this.hashChangeListener); @@ -113,38 +123,44 @@ const sfc = { methods: { // get the active container element, either the `job-step-logs` or the `job-log-list` in the `job-log-group` - getLogsContainer(idx) { - const el = this.$refs.logs[idx]; + getLogsContainer(stepIndex: number) { + const el = this.$refs.logs[stepIndex]; return el._stepLogsActiveContainer ?? el; }, // begin a log group - beginLogGroup(idx) { - const el = this.$refs.logs[idx]; - - const elJobLogGroup = document.createElement('div'); - elJobLogGroup.classList.add('job-log-group'); - - const elJobLogGroupSummary = document.createElement('div'); - elJobLogGroupSummary.classList.add('job-log-group-summary'); - - const elJobLogList = document.createElement('div'); - elJobLogList.classList.add('job-log-list'); - - elJobLogGroup.append(elJobLogGroupSummary); - elJobLogGroup.append(elJobLogList); + beginLogGroup(stepIndex: number, startTime: number, line: LogLine) { + const el = this.$refs.logs[stepIndex]; + const elJobLogGroupSummary = createElementFromAttrs('summary', {class: 'job-log-group-summary'}, + this.createLogLine(stepIndex, startTime, { + index: line.index, + timestamp: line.timestamp, + message: line.message.substring(LogLinePrefixGroup.length), + }), + ); + const elJobLogList = createElementFromAttrs('div', {class: 'job-log-list'}); + const elJobLogGroup = createElementFromAttrs('details', {class: 'job-log-group'}, + elJobLogGroupSummary, + elJobLogList, + ); + el.append(elJobLogGroup); el._stepLogsActiveContainer = elJobLogList; }, // end a log group - endLogGroup(idx) { - const el = this.$refs.logs[idx]; + endLogGroup(stepIndex: number, startTime: number, line: LogLine) { + const el = this.$refs.logs[stepIndex]; el._stepLogsActiveContainer = null; + el.append(this.createLogLine(stepIndex, startTime, { + index: line.index, + timestamp: line.timestamp, + message: line.message.substring(LogLinePrefixEndGroup.length), + })); }, // show/hide the step logs for a step - toggleStepLogs(idx) { + toggleStepLogs(idx: number) { this.currentJobStepsStates[idx].expanded = !this.currentJobStepsStates[idx].expanded; if (this.currentJobStepsStates[idx].expanded) { - this.loadJob(); // try to load the data immediately instead of waiting for next timer interval + this.loadJobForce(); // try to load the data immediately instead of waiting for next timer interval } }, // cancel a run @@ -156,62 +172,53 @@ const sfc = { POST(`${this.run.link}/approve`); }, - createLogLine(line, startTime, stepIndex) { - const div = document.createElement('div'); - div.classList.add('job-log-line'); - div.setAttribute('id', `jobstep-${stepIndex}-${line.index}`); - div._jobLogTime = line.timestamp; + createLogLine(stepIndex: number, startTime: number, line: LogLine) { + const lineNum = createElementFromAttrs('a', {class: 'line-num muted', href: `#jobstep-${stepIndex}-${line.index}`}, + String(line.index), + ); - const lineNumber = document.createElement('a'); - lineNumber.classList.add('line-num', 'muted'); - lineNumber.textContent = line.index; - lineNumber.setAttribute('href', `#jobstep-${stepIndex}-${line.index}`); - div.append(lineNumber); + const logTimeStamp = createElementFromAttrs('span', {class: 'log-time-stamp'}, + formatDatetime(new Date(line.timestamp * 1000)), // for "Show timestamps" + ); + + const logMsg = createElementFromAttrs('span', {class: 'log-msg'}); + logMsg.innerHTML = renderAnsi(line.message); + + const seconds = Math.floor(line.timestamp - startTime); + const logTimeSeconds = createElementFromAttrs('span', {class: 'log-time-seconds'}, + `${seconds}s`, // for "Show seconds" + ); - // for "Show timestamps" - const logTimeStamp = document.createElement('span'); - logTimeStamp.className = 'log-time-stamp'; - const date = new Date(parseFloat(line.timestamp * 1000)); - const timeStamp = formatDatetime(date); - logTimeStamp.textContent = timeStamp; toggleElem(logTimeStamp, this.timeVisible['log-time-stamp']); - // for "Show seconds" - const logTimeSeconds = document.createElement('span'); - logTimeSeconds.className = 'log-time-seconds'; - const seconds = Math.floor(parseFloat(line.timestamp) - parseFloat(startTime)); - logTimeSeconds.textContent = `${seconds}s`; toggleElem(logTimeSeconds, this.timeVisible['log-time-seconds']); - const logMessage = document.createElement('span'); - logMessage.className = 'log-msg'; - logMessage.innerHTML = renderAnsi(line.message); - div.append(logTimeStamp); - div.append(logMessage); - div.append(logTimeSeconds); - - return div; + return createElementFromAttrs('div', {id: `jobstep-${stepIndex}-${line.index}`, class: 'job-log-line'}, + lineNum, logTimeStamp, logMsg, logTimeSeconds, + ); }, - appendLogs(stepIndex, logLines, startTime) { + appendLogs(stepIndex: number, startTime: number, logLines: LogLine[]) { for (const line of logLines) { - // TODO: group support: ##[group]GroupTitle , ##[endgroup] const el = this.getLogsContainer(stepIndex); - el.append(this.createLogLine(line, startTime, stepIndex)); + if (line.message.startsWith(LogLinePrefixGroup)) { + this.beginLogGroup(stepIndex, startTime, line); + continue; + } else if (line.message.startsWith(LogLinePrefixEndGroup)) { + this.endLogGroup(stepIndex, startTime, line); + continue; + } + el.append(this.createLogLine(stepIndex, startTime, line)); } }, - async fetchArtifacts() { - const resp = await GET(`${this.actionsURL}/runs/${this.runIndex}/artifacts`); - return await resp.json(); - }, - - async deleteArtifact(name) { + async deleteArtifact(name: string) { if (!window.confirm(this.locale.confirmDeleteArtifact.replace('%s', name))) return; + // TODO: should escape the "name"? await DELETE(`${this.run.link}/artifacts/${name}`); - await this.loadJob(); + await this.loadJobForce(); }, - async fetchJob() { + async fetchJobData(abortController: AbortController) { const logCursors = this.currentJobStepsStates.map((it, idx) => { // cursor is used to indicate the last position of the logs // it's only used by backend, frontend just reads it and passes it back, it and can be any type. @@ -219,30 +226,27 @@ const sfc = { return {step: idx, cursor: it.cursor, expanded: it.expanded}; }); const resp = await POST(`${this.actionsURL}/runs/${this.runIndex}/jobs/${this.jobIndex}`, { + signal: abortController.signal, data: {logCursors}, }); return await resp.json(); }, + async loadJobForce() { + this.loadingAbortController?.abort(); + this.loadingAbortController = null; + await this.loadJob(); + }, + async loadJob() { - if (this.loading) return; + if (this.loadingAbortController) return; + const abortController = new AbortController(); + this.loadingAbortController = abortController; try { - this.loading = true; + const job = await this.fetchJobData(abortController); + if (this.loadingAbortController !== abortController) return; - let job, artifacts; - try { - [job, artifacts] = await Promise.all([ - this.fetchJob(), - this.fetchArtifacts(), // refresh artifacts if upload-artifact step done - ]); - } catch (err) { - if (err instanceof TypeError) return; // avoid network error while unloading page - throw err; - } - - this.artifacts = artifacts['artifacts'] || []; - - // save the state to Vue data, then the UI will be updated + this.artifacts = job.artifacts || []; this.run = job.state.run; this.currentJob = job.state.currentJob; @@ -254,26 +258,30 @@ const sfc = { } } // append logs to the UI - for (const logs of job.logs.stepsLog) { + for (const logs of job.logs.stepsLog ?? []) { // save the cursor, it will be passed to backend next time this.currentJobStepsStates[logs.step].cursor = logs.cursor; - this.appendLogs(logs.step, logs.lines, logs.started); + this.appendLogs(logs.step, logs.started, logs.lines); } if (this.run.done && this.intervalID) { clearInterval(this.intervalID); this.intervalID = null; } + } catch (e) { + // avoid network error while unloading page, and ignore "abort" error + if (e instanceof TypeError || abortController.signal.aborted) return; + throw e; } finally { - this.loading = false; + if (this.loadingAbortController === abortController) this.loadingAbortController = null; } }, - isDone(status) { + isDone(status: RunStatus) { return ['success', 'skipped', 'failure', 'cancelled'].includes(status); }, - isExpandable(status) { + isExpandable(status: RunStatus) { return ['success', 'running', 'failure', 'cancelled'].includes(status); }, @@ -281,7 +289,7 @@ const sfc = { if (this.menuVisible) this.menuVisible = false; }, - toggleTimeDisplay(type) { + toggleTimeDisplay(type: string) { this.timeVisible[`log-time-${type}`] = !this.timeVisible[`log-time-${type}`]; for (const el of this.$refs.steps.querySelectorAll(`.log-time-${type}`)) { toggleElem(el, this.timeVisible[`log-time-${type}`]); @@ -294,7 +302,7 @@ const sfc = { const outerEl = document.querySelector('.full.height'); const actionBodyEl = document.querySelector('.action-view-body'); const headerEl = document.querySelector('#navbar'); - const contentEl = document.querySelector('.page-content.repository'); + const contentEl = document.querySelector('.page-content'); const footerEl = document.querySelector('.page-footer'); toggleElem(headerEl, !this.isFullScreen); toggleElem(contentEl, !this.isFullScreen); @@ -332,7 +340,7 @@ export function initRepositoryActionView() { // TODO: the parent element's full height doesn't work well now, // but we can not pollute the global style at the moment, only fix the height problem for pages with this component - const parentFullHeight = document.querySelector('body > div.full.height'); + const parentFullHeight = document.querySelector('body > div.full.height'); if (parentFullHeight) parentFullHeight.style.paddingBottom = '0'; const view = createApp(sfc, { @@ -858,7 +866,7 @@ export function initRepositoryActionView() { white-space: nowrap; } -.job-step-section .job-step-logs .job-log-line .log-msg { +.job-step-logs .job-log-line .log-msg { flex: 1; word-break: break-all; white-space: break-spaces; @@ -884,15 +892,18 @@ export function initRepositoryActionView() { border-radius: 0; } -/* TODO: group support - -.job-log-group { - +.job-log-group .job-log-list .job-log-line .log-msg { + margin-left: 2em; } + .job-log-group-summary { - + position: relative; } -.job-log-list { -} */ +.job-log-group-summary > .job-log-line { + position: absolute; + inset: 0; + z-index: -1; /* to avoid hiding the triangle of the "details" element */ + overflow: hidden; +} From 0f18046df4a9560351fcd8a7d9c9a80adf10efc2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Fri, 6 Dec 2024 12:29:09 +0800 Subject: [PATCH 3/3] Refactor markdown render (#32728) Follow up recent render system refactoring PRs (split test code), and fine tune the math render (added some new cases) --- .../markdown/markdown_attention_test.go | 56 ++++++ .../markdown/markdown_benchmark_test.go | 25 +++ modules/markup/markdown/markdown_math_test.go | 163 ++++++++++++++++++ modules/markup/markdown/markdown_test.go | 131 -------------- modules/markup/markdown/math/block_parser.go | 11 +- modules/markup/markdown/math/inline_parser.go | 48 +++--- 6 files changed, 268 insertions(+), 166 deletions(-) create mode 100644 modules/markup/markdown/markdown_attention_test.go create mode 100644 modules/markup/markdown/markdown_benchmark_test.go create mode 100644 modules/markup/markdown/markdown_math_test.go diff --git a/modules/markup/markdown/markdown_attention_test.go b/modules/markup/markdown/markdown_attention_test.go new file mode 100644 index 0000000000..f6ec775b2c --- /dev/null +++ b/modules/markup/markdown/markdown_attention_test.go @@ -0,0 +1,56 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markdown_test + +import ( + "strings" + "testing" + + "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/markup/markdown" + "code.gitea.io/gitea/modules/svg" + + "github.com/stretchr/testify/assert" + "golang.org/x/text/cases" + "golang.org/x/text/language" +) + +func TestAttention(t *testing.T) { + defer svg.MockIcon("octicon-info")() + defer svg.MockIcon("octicon-light-bulb")() + defer svg.MockIcon("octicon-report")() + defer svg.MockIcon("octicon-alert")() + defer svg.MockIcon("octicon-stop")() + + renderAttention := func(attention, icon string) string { + tmpl := `

{Attention}

` + tmpl = strings.ReplaceAll(tmpl, "{attention}", attention) + tmpl = strings.ReplaceAll(tmpl, "{icon}", icon) + tmpl = strings.ReplaceAll(tmpl, "{Attention}", cases.Title(language.English).String(attention)) + return tmpl + } + + test := func(input, expected string) { + result, err := markdown.RenderString(markup.NewTestRenderContext(), input) + assert.NoError(t, err) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(result))) + } + + test(` +> [!NOTE] +> text +`, renderAttention("note", "octicon-info")+"\n

text

\n
") + + test(`> [!note]`, renderAttention("note", "octicon-info")+"\n") + test(`> [!tip]`, renderAttention("tip", "octicon-light-bulb")+"\n") + test(`> [!important]`, renderAttention("important", "octicon-report")+"\n") + test(`> [!warning]`, renderAttention("warning", "octicon-alert")+"\n") + test(`> [!caution]`, renderAttention("caution", "octicon-stop")+"\n") + + // escaped by mdformat + test(`> \[!NOTE\]`, renderAttention("note", "octicon-info")+"\n") + + // legacy GitHub style + test(`> **warning**`, renderAttention("warning", "octicon-alert")+"\n") +} diff --git a/modules/markup/markdown/markdown_benchmark_test.go b/modules/markup/markdown/markdown_benchmark_test.go new file mode 100644 index 0000000000..0f7e3eea6f --- /dev/null +++ b/modules/markup/markdown/markdown_benchmark_test.go @@ -0,0 +1,25 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markdown_test + +import ( + "testing" + + "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/markup/markdown" +) + +func BenchmarkSpecializedMarkdown(b *testing.B) { + // 240856 4719 ns/op + for i := 0; i < b.N; i++ { + markdown.SpecializedMarkdown(&markup.RenderContext{}) + } +} + +func BenchmarkMarkdownRender(b *testing.B) { + // 23202 50840 ns/op + for i := 0; i < b.N; i++ { + _, _ = markdown.RenderString(markup.NewTestRenderContext(), "https://example.com\n- a\n- b\n") + } +} diff --git a/modules/markup/markdown/markdown_math_test.go b/modules/markup/markdown/markdown_math_test.go new file mode 100644 index 0000000000..0e5adeeac8 --- /dev/null +++ b/modules/markup/markdown/markdown_math_test.go @@ -0,0 +1,163 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markdown + +import ( + "strings" + "testing" + + "code.gitea.io/gitea/modules/markup" + + "github.com/stretchr/testify/assert" +) + +func TestMathRender(t *testing.T) { + const nl = "\n" + testcases := []struct { + testcase string + expected string + }{ + { + "$a$", + `

a

` + nl, + }, + { + "$ a $", + `

a

` + nl, + }, + { + "$a$ $b$", + `

a b

` + nl, + }, + { + `\(a\) \(b\)`, + `

a b

` + nl, + }, + { + `$a$.`, + `

a.

` + nl, + }, + { + `.$a$`, + `

.$a$

` + nl, + }, + { + `$a a$b b$`, + `

$a a$b b$

` + nl, + }, + { + `a a$b b`, + `

a a$b b

` + nl, + }, + { + `a$b $a a$b b$`, + `

a$b $a a$b b$

` + nl, + }, + { + "a$x$", + `

a$x$

` + nl, + }, + { + "$x$a", + `

$x$a

` + nl, + }, + { + "$a$ ($b$) [$c$] {$d$}", + `

a (b) [$c$] {$d$}

` + nl, + }, + { + "$$a$$", + `
a
` + nl, + }, + { + "$$a$$ test", + `

a test

` + nl, + }, + { + "test $$a$$", + `

test a

` + nl, + }, + { + "foo $x=\\$$ bar", + `

foo x=\$ bar

` + nl, + }, + } + + for _, test := range testcases { + t.Run(test.testcase, func(t *testing.T) { + res, err := RenderString(markup.NewTestRenderContext(), test.testcase) + assert.NoError(t, err) + assert.Equal(t, test.expected, string(res)) + }) + } +} + +func TestMathRenderBlockIndent(t *testing.T) { + testcases := []struct { + name string + testcase string + expected string + }{ + { + "indent-0", + ` +\[ +\alpha +\] +`, + `

+\alpha
+
+`, + }, + { + "indent-1", + ` + \[ + \alpha + \] +`, + `

+\alpha
+
+`, + }, + { + "indent-2", + ` + \[ + \alpha + \] +`, + `

+\alpha
+
+`, + }, + { + "indent-0-oneline", + `$$ x $$ +foo`, + `
 x
+

foo

+`, + }, + { + "indent-3-oneline", + ` $$ x $$ +foo`, + `
 x
+

foo

+`, + }, + } + + for _, test := range testcases { + t.Run(test.name, func(t *testing.T) { + res, err := RenderString(markup.NewTestRenderContext(), strings.ReplaceAll(test.testcase, "", " ")) + assert.NoError(t, err) + assert.Equal(t, test.expected, string(res), "unexpected result for test case:\n%s", test.testcase) + }) + } +} diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index 22ab39ebfa..7a09be8665 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -13,13 +13,10 @@ import ( "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/svg" "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" - "golang.org/x/text/cases" - "golang.org/x/text/language" ) const ( @@ -386,81 +383,6 @@ func TestColorPreview(t *testing.T) { } } -func TestMathBlock(t *testing.T) { - const nl = "\n" - testcases := []struct { - testcase string - expected string - }{ - { - "$a$", - `

a

` + nl, - }, - { - "$ a $", - `

a

` + nl, - }, - { - "$a$ $b$", - `

a b

` + nl, - }, - { - `\(a\) \(b\)`, - `

a b

` + nl, - }, - { - `$a$.`, - `

a.

` + nl, - }, - { - `.$a$`, - `

.$a$

` + nl, - }, - { - `$a a$b b$`, - `

$a a$b b$

` + nl, - }, - { - `a a$b b`, - `

a a$b b

` + nl, - }, - { - `a$b $a a$b b$`, - `

a$b $a a$b b$

` + nl, - }, - { - "a$x$", - `

a$x$

` + nl, - }, - { - "$x$a", - `

$x$a

` + nl, - }, - { - "$$a$$", - `
a
` + nl, - }, - { - "$a$ ($b$) [$c$] {$d$}", - `

a (b) [$c$] {$d$}

` + nl, - }, - { - "$$a$$ test", - `

a test

` + nl, - }, - { - "test $$a$$", - `

test a

` + nl, - }, - } - - for _, test := range testcases { - res, err := markdown.RenderString(markup.NewTestRenderContext(), test.testcase) - assert.NoError(t, err, "Unexpected error in testcase: %q", test.testcase) - assert.Equal(t, template.HTML(test.expected), res, "Unexpected result in testcase %q", test.testcase) - } -} - func TestTaskList(t *testing.T) { testcases := []struct { testcase string @@ -551,56 +473,3 @@ space

assert.NoError(t, err) assert.Equal(t, expected, string(result)) } - -func TestAttention(t *testing.T) { - defer svg.MockIcon("octicon-info")() - defer svg.MockIcon("octicon-light-bulb")() - defer svg.MockIcon("octicon-report")() - defer svg.MockIcon("octicon-alert")() - defer svg.MockIcon("octicon-stop")() - - renderAttention := func(attention, icon string) string { - tmpl := `

{Attention}

` - tmpl = strings.ReplaceAll(tmpl, "{attention}", attention) - tmpl = strings.ReplaceAll(tmpl, "{icon}", icon) - tmpl = strings.ReplaceAll(tmpl, "{Attention}", cases.Title(language.English).String(attention)) - return tmpl - } - - test := func(input, expected string) { - result, err := markdown.RenderString(markup.NewTestRenderContext(), input) - assert.NoError(t, err) - assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(result))) - } - - test(` -> [!NOTE] -> text -`, renderAttention("note", "octicon-info")+"\n

text

\n
") - - test(`> [!note]`, renderAttention("note", "octicon-info")+"\n") - test(`> [!tip]`, renderAttention("tip", "octicon-light-bulb")+"\n") - test(`> [!important]`, renderAttention("important", "octicon-report")+"\n") - test(`> [!warning]`, renderAttention("warning", "octicon-alert")+"\n") - test(`> [!caution]`, renderAttention("caution", "octicon-stop")+"\n") - - // escaped by mdformat - test(`> \[!NOTE\]`, renderAttention("note", "octicon-info")+"\n") - - // legacy GitHub style - test(`> **warning**`, renderAttention("warning", "octicon-alert")+"\n") -} - -func BenchmarkSpecializedMarkdown(b *testing.B) { - // 240856 4719 ns/op - for i := 0; i < b.N; i++ { - markdown.SpecializedMarkdown(&markup.RenderContext{}) - } -} - -func BenchmarkMarkdownRender(b *testing.B) { - // 23202 50840 ns/op - for i := 0; i < b.N; i++ { - _, _ = markdown.RenderString(markup.NewTestRenderContext(), "https://example.com\n- a\n- b\n") - } -} diff --git a/modules/markup/markdown/math/block_parser.go b/modules/markup/markdown/math/block_parser.go index a23f48b637..f31cfb09ad 100644 --- a/modules/markup/markdown/math/block_parser.go +++ b/modules/markup/markdown/math/block_parser.go @@ -54,21 +54,19 @@ func (b *blockParser) Open(parent ast.Node, reader text.Reader, pc parser.Contex idx := bytes.Index(line[pos+2:], endBytes) if idx >= 0 { // for case $$ ... $$ any other text - for i := pos + idx + 4; i < len(line); i++ { + for i := pos + 2 + idx + 2; i < len(line); i++ { if line[i] != ' ' && line[i] != '\n' { return nil, parser.NoChildren } } - segment.Stop = segment.Start + idx + 2 - reader.Advance(segment.Len() - 1) - segment.Start += 2 + segment.Start += pos + 2 + segment.Stop = segment.Start + idx node.Lines().Append(segment) node.Closed = true return node, parser.Close | parser.NoChildren } - reader.Advance(segment.Len() - 1) - segment.Start += 2 + segment.Start += pos + 2 node.Lines().Append(segment) return node, parser.NoChildren } @@ -103,7 +101,6 @@ func (b *blockParser) Continue(node ast.Node, reader text.Reader, pc parser.Cont pos, padding := util.IndentPosition(line, 0, block.Indent) seg := text.NewSegmentPadding(segment.Start+pos, segment.Stop, padding) node.Lines().Append(seg) - reader.AdvanceAndSetPadding(segment.Stop-segment.Start-pos-1, padding) return parser.Continue | parser.NoChildren } diff --git a/modules/markup/markdown/math/inline_parser.go b/modules/markup/markdown/math/inline_parser.go index b11195d551..56ae3d57eb 100644 --- a/modules/markup/markdown/math/inline_parser.go +++ b/modules/markup/markdown/math/inline_parser.go @@ -26,7 +26,6 @@ var defaultDualDollarParser = &inlineParser{ end: []byte{'$', '$'}, } -// NewInlineDollarParser returns a new inline parser func NewInlineDollarParser() parser.InlineParser { return defaultInlineDollarParser } @@ -40,7 +39,6 @@ var defaultInlineBracketParser = &inlineParser{ end: []byte{'\\', ')'}, } -// NewInlineDollarParser returns a new inline parser func NewInlineBracketParser() parser.InlineParser { return defaultInlineBracketParser } @@ -81,35 +79,29 @@ func (parser *inlineParser) Parse(parent ast.Node, block text.Reader, pc parser. opener := len(parser.start) // Now look for an ending line - ender := opener - for { - pos := bytes.Index(line[ender:], parser.end) - if pos < 0 { - return nil - } - - ender += pos - - // Now we want to check the character at the end of our parser section - // that is ender + len(parser.end) and check if char before ender is '\' - pos = ender + len(parser.end) - if len(line) <= pos { + ender := -1 + for i := opener; i < len(line); i++ { + if bytes.HasPrefix(line[i:], parser.end) { + succeedingCharacter := byte(0) + if i+len(parser.end) < len(line) { + succeedingCharacter = line[i+len(parser.end)] + } + // check valid ending character + isValidEndingChar := isPunctuation(succeedingCharacter) || isBracket(succeedingCharacter) || + succeedingCharacter == ' ' || succeedingCharacter == '\n' || succeedingCharacter == 0 + if !isValidEndingChar { + break + } + ender = i break } - suceedingCharacter := line[pos] - // check valid ending character - if !isPunctuation(suceedingCharacter) && - !(suceedingCharacter == ' ') && - !(suceedingCharacter == '\n') && - !isBracket(suceedingCharacter) { - return nil + if line[i] == '\\' { + i++ + continue } - if line[ender-1] != '\\' { - break - } - - // move the pointer onwards - ender += len(parser.end) + } + if ender == -1 { + return nil } block.Advance(opener)