-
Notifications
You must be signed in to change notification settings - Fork 428
feat(example): refactor p/demo/blog #4324
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
base: master
Are you sure you want to change the base?
Conversation
🛠 PR Checks Summary🔴 Pending initial approval by a review team member, or review from tech-staff Manual Checks (for Reviewers):
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) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…-blog feat(blog): keep main structs to settle the structure of all types
…-blog feat(blog): add base rendering for preview of posts
Prefix string | ||
Posts *avl.Tree | ||
PostId seqid.ID | ||
DisplayMode string // Grid or List |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to have this as an option in the render func, no? you then allow the user to render it however they want instead of having to toggle it :)
Also, imagine the blog instance is saved somewhere else except the specific realm its made in, maybe that other instance/realm wants to do something else to it, and with a toggle like this it's dependent on who has the power to change the display mode
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion, but just to clarify then (seeing as though this would change the Render function):
Are you thinking of moving away entirely from how p/demo/blog
works (with Render(path string) and internal route handling via mux
), and instead have rendering logic be completely controlled externally — like the caller deciding list/grid and rendering themselves per route?
Or do you still see the value in keeping built-in routing/rendering (like RenderPosts, RenderPost, etc.) with query params like ?mode=list)?
(if that makes sense)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or do you still see the value in keeping built-in routing/rendering (like RenderPosts, RenderPost, etc.) with query params like ?mode=list)?
This :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
out := md.H4(b.Mention("commenter", user)) + "\n\n" | ||
out += comment.Content() + "\n\n" | ||
out += md.Italic(formatTime(comment.CreatedAt(), options.TimeFormat)) + "\n\n" | ||
out += "in " + md.Link(p.Title(), b.Prefix()+":posts/"+p.Slug()) + "\n\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the commit to address this issue. It's better, but still not quite right. I add a post where the slug is "slug/1".
In the post, it shows the escaped "slug%2F1" . When you hover over the title, the URL is "http://127.0.0.1:8888/r/lou/blog:posts/slug/1" and when you click the title, it shows a 404 error. Maybe the URL should be "http://127.0.0.1:8888/r/lou/blog:posts/slug%252F1". Have the do the escaping at the right level.

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, didn't see this:
2d465d4
is this what you meant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The realm page has options to filter the blog posts. As we discussed in the dev call, the Render function has a CPU limit. In this video, I did a stress test to demonstrate this limit. I also discuss possible mitigations, including warning the user when getting close to the limit. |
Hey @louonezime, can you please take a look at the CI and why it's failing? |
This PR introduces a new query package that provides reusable helpers for working with URL query strings. It is designed to simplify parsing, updating, and reconstructing query parameters in a clean and generic way. This logic was originally embedded in the `blog` package (in gnolang#4324), but has now been extracted into its own utility package to promote reusability and separation of concerns. - ParseQueryParams(key, rawURL) → get a single query parameter value - ParseQueryMap(rawURL) → return all query parameters as a map[string]string - UpdateQuery(rawURL, key, value) → set or overwrite a single query param - UpdateQueryMulti(rawURL, map[string]string) → update multiple query params at once - DeleteQuery(rawURL, key) → remove a parameter from the query string *UPDATE* - `GetQueryValues` – Get all values for a given key. - `GetQueryFirstValue` – Get the first value for a given key. - `HasQueryKey` – Check if a key exists in the query. - `GetQueryValueFromURL` – Retrieve a query value from a raw URL. - `ParseQueryFirstValuesMap` – Parse a URL into a map of first values. - `UpdateQueryValue` – Set a single value for a key. - `UpdateQueryValues` – Set multiple values for a single key. - `UpdateQueryFirstValues` – Set single values for multiple keys. - `UpdateQueryAllValues` – Set multiple values for multiple keys. - `SetQueries` – Replace or add keys with single values. - `SetQueriesMulti` – Replace or add keys with multiple values. - `DeleteQuery` – Remove a specific key from the query. - `ResetQuery` – Remove all query parameters from the URL.
This PR is linked to the #4088 for scratching the old
/p/demo/blog
to make it better but still have it be compatible with thegnoblo-cli
.This is currently in
/p/lou/blog
as to start with a completely fresh package, the git diff is nicer to read this way.These are the features:
Base:
mux
:tag/<tagname>
?sort=alpha&order=asc
?sort=update&order=asc
Added/modified:
?start=<time>&end=<time>
:author/<authorname>
?mode=grid
or?mode=list
seq.id
as indexesr/sys/users
registry.Further (not mentioned in issue):
:commenter/<commentername>
?sort=recent&order=asc
Mention
method to simulate a link to a specific author/ commenter (will link to :author or :commenter)Package created to support: