The Wayback Machine - https://web.archive.org/web/20220503002636/https://github.com/go-gitea/gitea/pull/15175
Skip to content
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

Jump to bottom

Refactor renders #15175

Merged
merged 27 commits into from Apr 19, 2021
Merged

Refactor renders #15175

merged 27 commits into from Apr 19, 2021

Conversation

lunny
Copy link
Member

@lunny lunny commented Mar 27, 2021 •
edited

This PR refactor the Render system.

  • Rename Parser to Renderer because the previous name is confusing
  • Changed Render function signature of Renderer to accept input io.Reader and output io.Writer and return error
  • Reduce memory usage when possible.

@lunny lunny added this to the 1.15.0 milestone Mar 27, 2021
@lunny lunny force-pushed the lunny/refactor_render branch 4 times, most recently from 7407a70 to d99a6f7 Compare Mar 31, 2021
@lunny lunny changed the title WIP: Refactor renders Refactor renders Mar 31, 2021
6543
6543 reviewed Apr 1, 2021
View changes
modules/markup/csv/csv.go Outdated Show resolved Hide resolved
modules/markup/csv/csv.go Outdated Show resolved Hide resolved
modules/markup/csv/csv_test.go Show resolved Hide resolved
modules/markup/orgmode/orgmode.go Outdated Show resolved Hide resolved
routers/repo/view.go Outdated Show resolved Hide resolved
6543
6543 reviewed Apr 1, 2021
View changes
modules/markup/markdown/markdown.go
@@ -184,18 +185,26 @@ func actualRender(body []byte, urlPrefix string, metas map[string]string, wikiMa
_ = lw.CloseWithError(fmt.Errorf("%v", err))
}()

pc := NewGiteaParseContext(urlPrefix, metas, wikiMarkdown)
if err := converter.Convert(giteautil.NormalizeEOL(body), lw, parser.WithContext(pc)); err != nil {
// FIXME: Don't read all to memory, but goldmark doesn't support
Copy link
Member

@6543 6543 Apr 1, 2021

Choose a reason for hiding this comment

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

exist an upstream issue for this?

Copy link
Member Author

@lunny lunny Apr 1, 2021 •
edited

Choose a reason for hiding this comment

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

I think yes, I cannot find any function to allow io.Reader

Copy link
Member

@6543 6543 Apr 1, 2021

Choose a reason for hiding this comment

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

yuin/goldmark#209

@lunny lunny force-pushed the lunny/refactor_render branch 2 times, most recently from 30a8387 to 8c88a22 Compare Apr 1, 2021
@lunny lunny removed the status/wip label Apr 1, 2021
6543
6543 approved these changes Apr 1, 2021
View changes
@6543
Copy link

@6543 6543 commented Apr 3, 2021

@lunny can you resolve conflicts .. made by (#15072)

@lunny lunny force-pushed the lunny/refactor_render branch 2 times, most recently from 7aa0736 to 4e0eda6 Compare Apr 17, 2021
@codecov-commenter
Copy link

@codecov-commenter codecov-commenter commented Apr 17, 2021 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (master@7417628). Click here to learn what that means.
The diff coverage is 55.93%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #15175   +/-   ##
=========================================
  Coverage          ?   43.82%           
=========================================
  Files             ?      678           
  Lines             ?    81730           
  Branches          ?        0           
=========================================
  Hits              ?    35815           
  Misses            ?    40082           
  Partials          ?     5833
Impacted Files Coverage Δ
modules/notification/mail/mail.go 33.67% <0.00%> (ø)
modules/setting/markup.go 1.58% <0.00%> (ø)
routers/org/home.go 61.00% <0.00%> (ø)
routers/repo/compare.go 40.93% <0.00%> (ø)
routers/repo/lfs.go 0.00% <0.00%> (ø)
routers/repo/milestone.go 0.00% <0.00%> (ø)
routers/user/profile.go 42.17% <0.00%> (ø)
modules/markup/external/external.go 2.00% <5.00%> (ø)
modules/markup/csv/csv.go 36.23% <22.22%> (ø)
routers/repo/release.go 23.19% <28.57%> (ø)
... and 20 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7417628...1eeb587. Read the comment docs.

zeripath
zeripath reviewed Apr 17, 2021
View changes
modules/notification/mail/mail.go Outdated Show resolved Hide resolved
modules/notification/mail/mail.go Outdated Show resolved Hide resolved
modules/util/util.go Outdated Show resolved Hide resolved
routers/repo/wiki.go Show resolved Hide resolved
@lunny
Copy link
Author

@lunny lunny commented Apr 17, 2021 •
edited

Since we have switched to yuin/goldmark, the NormalEOF is unnecssary so I have removed the invoke about that.

@techknowlogick
Copy link

@techknowlogick techknowlogick commented Apr 19, 2021

🚀

@techknowlogick techknowlogick merged commit 9d99f6a into go-gitea:master Apr 19, 2021
2 checks passed
@lunny lunny deleted the lunny/refactor_render branch Apr 19, 2021
6543 added a commit to 6543-forks/gitea that referenced this issue Apr 22, 2021
@6543
Fix go-fuzz
df6a8bc 
followup of go-gitea#15175
@6543 6543 mentioned this pull request Apr 22, 2021
Merged
lunny pushed a commit that referenced this issue Apr 23, 2021
@6543
Fix go-fuzz (#15596)
bee8ce3 
* Fix go-fuzz

followup of #15175

* simplify

* enhance
@lunny lunny mentioned this pull request Apr 28, 2021
Merged
@go-gitea go-gitea locked and limited conversation to collaborators Jun 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KN4CK3R KN4CK3R

@zeripath zeripath

@6543 6543

Assignees
No one assigned
Labels
kind/refactor lgtm/done
Milestone
1.15.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@lunny @6543 @codecov-commenter @techknowlogick @KN4CK3R @zeripath @GiteaBot