gno icon indicating copy to clipboard operation
gno copied to clipboard

feat: [gnoweb] Allow goldmark extentions

Open stackdump opened this issue 5 months ago • 2 comments

Revisit https://github.com/gnolang/gno/pull/4186 - adding a better means to customize markdown rendering

Description

We are introducing a mechanism to allow custom Markdown rendering in gnoweb through an optional build tag and injection of a custom renderer factory.

Goals

Add setup_gnomark.go behind the gnomark build tag to register a custom Markdown renderer using Goldmark.

Allow external packages to override the default MarkdownRenderer behavior by injecting a RenderFactory.

Decouple renderer instantiation from hardcoded logic in NewRouter.

Changes

New file: contribs/gnodev/cmd/gnodev/setup_gnomark.go

Registers a RenderFactory for Markdown rendering using goldmark.

Uses gnoweb.SetRenderFactory() in init() when gnomark tag is set.

gno.land/pkg/gnoweb/app.go

Adds RenderFactory as a configurable function.

Introduces SetRenderFactory() and a default implementation DefaultMarkdown().

Replaces inline Goldmark setup in NewRouter() with the factory.

gno.land/pkg/gnoweb/markdown.go

Renames markdown → Markdown and logger → Logger (exported for custom usage).

Adjusts Render() to use the exported fields, enabling injected implementations to be compatible.

Motivation

This change provides a clean extension point for advanced Markdown rendering, such as:

Custom extensions (e.g., task lists, Gno-specific syntax)

Developer-only builds with preview rendering

Future toggles for experimental render pipelines

stackdump avatar Jun 22 '25 01:06 stackdump

🛠 PR Checks Summary

🔴 Changes related to gnoweb must be reviewed by its codeowners

Manual Checks (for Reviewers):
  • [ ] IGNORE the bot requirements for this PR (force green CI check)
  • [ ] The pull request description provides enough details
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info) 🔴 Changes related to gnoweb must be reviewed by its codeowners

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: stackdump/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Changes related to gnoweb must be reviewed by its codeowners

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 A changed file matches this pattern: ^gno.land/pkg/gnoweb/ (filename: gno.land/pkg/gnoweb/app.go)

Then

🔴 Requirement not satisfied
└── 🔴 Or
    ├── 🔴 Or
    │   ├── 🔴 And
    │   │   ├── 🔴 Pull request author is user: alexiscolin
    │   │   └── 🔴 This user reviewed pull request: gfanton (with state "APPROVED")
    │   └── 🔴 And
    │       ├── 🔴 Pull request author is user: gfanton
    │       └── 🔴 This user reviewed pull request: alexiscolin (with state "APPROVED")
    └── 🔴 And
        ├── 🟢 Not (🔴 Pull request author is user: alexiscolin)
        ├── 🟢 Not (🔴 Pull request author is user: gfanton)
        └── 🔴 Or
            ├── 🔴 This user reviewed pull request: alexiscolin (with state "APPROVED")
            └── 🔴 This user reviewed pull request: gfanton (with state "APPROVED")

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission
The pull request description provides enough details

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 Not (🔴 Pull request author is a member of the team: core-contributors)
    └── 🟢 Not (🔴 Pull request author is user: dependabot[bot])

Can be checked by

  • team core-contributors

Gno2D2 avatar Jun 22 '25 01:06 Gno2D2

Codecov Report

Attention: Patch coverage is 35.00000% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
gno.land/pkg/gnoweb/app.go 26.66% 9 Missing and 2 partials :warning:
gno.land/pkg/gnoweb/markdown.go 60.00% 1 Missing and 1 partial :warning:

:loudspeaker: Thoughts on this report? Let us know!

codecov[bot] avatar Jun 22 '25 01:06 codecov[bot]

I don't believe this is the right approach. But don't make me wrong; I'm definitely open to extending or overriding gnoweb's rendering capability, but not in this manner.

In my opignon we should:

  • pass rendering as an app options (im actually adding this in #4410)
  • move most of gnodev cmd code in an app package, so external package can import it and extend/override it.
    • this app package could then take a gnoweb factory as an options, to create custom instance of gnoweb.

I need to think more about how to implement this. If the gnodev's cmd App is exposed, it will require some reworking.

In the meantime, don't hesitate to reach me out directly, especially if this is blocking you somehow.

It sounds like the above changes will provide a clean way for me to integrate in the future.

Let me know if I can help the refactor effort.

stackdump avatar Jun 26 '25 16:06 stackdump

closing - will watch progress on https://github.com/gnolang/gno/pull/4410

stackdump avatar Jun 26 '25 16:06 stackdump