-
Notifications
You must be signed in to change notification settings - Fork 396
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
fix(gnoweb): transform relative args links #3981
base: master
Are you sure you want to change the base?
Conversation
🛠 PR Checks SummaryAll Automated Checks passed. ✅ 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 ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
Even simpler 👍 Thank you @gfanton |
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.
Should we document this somewhere? Otherwise, it looks good! Thanks!
In r/docs/markdown we can have a section for links, ie abs vs relative vs special chars |
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 had a similar idea at one point but then decided to focus on creating good p/ libraries to manage this for us. I still believe this idea could be very beneficial, but I prefer blocking so we can discuss it with more people during a review meeting.
@moul I don't know if you are referring to these two gno packages or other things:
But none of them fixes the problem we are having with relative paths not working. It is impossible to fix at gno code level because for that you must know from where your code is called. Implementing the proxy pattern, I needed to delegate the Render call to a struct. That Render method can be called from gno.land/r/gov/dao or gno.land/r/gov/dao/v3/impl. Not having relative paths working makes it impossible to properly write the markdown code for that case. In any case, I think we need relative paths, and we don't have them. What I saw was people having to hardcode the name of the package realm on their code to have it working properly. Examples: |
I was referring to this one: https://github.com/gnolang/gno/blob/master/examples/gno.land/p/moul/realmpath/realmpath.gno I don't understand how we can say, "you must know where your code is called," while also mentioning "relative links." If something is relative, it pertains to the current rendering realm, which is the one from which you are executing Render(). Is that correct? Perhaps realmpath should, like the txlink package, allow specifying an arbitrary realm as the caller. Alternatively, it could have another constructor that accepts previousrealm. I agree that this approach seems more complex than necessary. However, using relative links in a context where we can create rendering helpers appears more dangerous than explicitly creating full paths. That said, I think it's reasonable to view a realm as akin to a domain name. In this context, the notion of a relative link makes sense if we consider each realm to be a unique domain. Can we explore how using realmpath (or another solution) could address your case before considering this PR? |
Description
This PR fixes issue #3865 by implementing proper transformation of relative links that start with ":" into full gnoweb URLs. This ensures proper URL resolution and maintains the gno.land URL structure.
Changes
TransformRelArgsURL
function to handle relative links starting with ":"Example
Fixes: #3865