Fixes gemini relative links to work for all relative link types #168

sloum · 2020-05-26T22:18:14Z

sloum commented

2020-05-26 22:18:14 +00:00

Also adds comments to the handleRelativeUrl method to make this a little more clear next time someone needs to wade into these waters.

This fix has been verified by testing:

gemini://rawtext.club/~sloum/musicmachine/test.gmi
- There are four test cases here for these types of relative links: ./file.gmi , ../file.gmi , /file.gmi , file.gmi
- All four are now working as expected
gemini://makeworld.gq/gemlog/
- This was the capsule that was originally given as an example of the buggy behavior
- The first link on the page is relative and should be for the same dir, but was being assigned to the root dir instead.

This PR is intended to close #163

Also adds comments to the `handleRelativeUrl` method to make this a little more clear next time someone needs to wade into these waters. This fix has been verified by testing: - gemini://rawtext.club/~sloum/musicmachine/test.gmi - There are four test cases here for these types of relative links: ./file.gmi , ../file.gmi , /file.gmi , file.gmi - All four are now working as expected - gemini://makeworld.gq/gemlog/ - This was the capsule that was originally given as an example of the buggy behavior - The first link on the page is relative and should be for the same dir, but was being assigned to the root dir instead. This PR is intended to close #163

🚀 1

sloum added the

bug

urgent

gemini

labels 2020-05-26 22:18:14 +00:00

sloum referenced this pull request

2020-05-26 22:20:16 +00:00

Urgent: All relative links are applied to hostname root #163

makeworld commented

2020-05-26 23:38:36 +00:00

First-time contributor

Can confirm it works, thanks!

makeworld commented

2020-05-26 23:40:52 +00:00

First-time contributor

Is there a reason why ResolveReference can't be used in all link cases to figure out the new path? This looks much simpler, but obviously I don't know much about how Bombadillo work internally (yet).

Is there a reason why [ResolveReference](https://golang.org/pkg/net/url/#URL.ResolveReference) can't be used in all link cases to figure out the new path? This looks much simpler, but obviously I don't know much about how Bombadillo work internally (yet).

sloum commented

2020-05-27 04:54:07 +00:00

I did not know it existed :) I have not used the net/url stuff almost at all in this project other than at your suggestion.

Bombadillo started primarily as, and continues to be, a learning project for me to explore things I was/am curious about (terminal handling, writing a terminal ui style application without ncurses, makefiles, etc) while ending up being something truly useful for me.

At work my boss is a big anti-nih (not-invented-here) guy. So as a result we barely write any code of our own and just string libraries together. I really do not like that kind of coding (it is useful for a business, but I dont learn much useful from it). So due to that my approach to bombadillo has often been to write... which is sometimes buggy, but I almost always learn something from it. That said, in a case like this I have no objection to leveraging something in the standard library.

I have rewritten the, now much much shorter, method for handling relative references leveraging the mentioned function. If you get a chance to give it a try and verify that it is working as you would expect I would greatly appreciate it!

I did not know it existed :) I have not used the `net/url` stuff almost at all in this project other than at your suggestion. Bombadillo started primarily as, and continues to be, a learning project for me to explore things I was/am curious about (terminal handling, writing a terminal ui style application without ncurses, makefiles, etc) while ending up being something truly useful for me. At work my boss is a big _anti-nih_ (not-invented-here) guy. So as a result we barely write any code of our own and just string libraries together. I really do not like that kind of coding (it is useful for a business, but I dont learn much useful from it). So due to that my approach to bombadillo has often been to write... which is sometimes buggy, but I almost always learn something from it. That said, in a case like this I have no objection to leveraging something in the standard library. I have rewritten the, now much much shorter, method for handling relative references leveraging the mentioned function. If you get a chance to give it a try and verify that it is working as you would expect I would greatly appreciate it!

makeworld commented

2020-05-27 12:10:51 +00:00

First-time contributor

Glad I was able to help! Just tried it out, it seems to work well but link 2 on your test.gmi page doesn't work. That's not the client but the page itself though, because a link like /spacewalk.gmi will resolve to rawtext.club/spacewalk.gmi, which doesn't exist. Maybe change that link to /social_contract.gmi?

Glad I was able to help! Just tried it out, it seems to work well but link 2 on your `test.gmi` page doesn't work. That's not the client but the page itself though, because a link like `/spacewalk.gmi` will resolve to `rawtext.club/spacewalk.gmi`, which doesn't exist. Maybe change that link to `/social_contract.gmi`?

sloum commented

2020-05-27 14:28:25 +00:00

Cool! Glad to hear it worked. Nah, I'm gonna delete the doc. It was just a temp way to see changes. I never actually followed any links on the page so there was no need for them to be valid links (I just used :check 2, etc).

Cool! Glad to hear it worked. Nah, I'm gonna delete the doc. It was just a temp way to see changes. I never actually followed any links on the page so there was no need for them to be valid links (I just used `:check 2`, etc).

sloum referenced this issue from a commit

2020-05-28 04:50:13 +00:00

Merge pull request 'Fixes gemini relative links to work for all relative link types' (#168) from fix-relative-links into release-2.3.1 Since this has been reviewed as functional I am merging into the release branch. There will be another opportunity to review, if need be, for the release branch when a PR is made into `develop`.

sloum closed this pull request

2020-05-28 04:50:14 +00:00

sloum deleted branch fix-relative-links

2020-05-28 04:50:27 +00:00

Sign in to join this conversation.

No reviewers