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

Handle /c/community@host URLs #583

Merged
merged 2 commits into from
Jun 15, 2023

Conversation

beatgammit
Copy link
Contributor

This is a partial fix to #556, but it at least prevents crashing when an unknown URL type is encountered. There is still some work to do, since the markdown plugin interprets the following as mailto: urls:

  • @user@instance
  • !community@instance

I'm not sure what the supported URL types are intended to be, so I didn't bother implementing a plugin.

@beatgammit beatgammit force-pushed the feat/fix-crash-on-community-url branch from 16f7877 to b287be8 Compare June 13, 2023 07:22
@beatgammit
Copy link
Contributor Author

Also partial fix for #454

app/src/main/java/com/jerboa/Utils.kt Outdated Show resolved Hide resolved
app/src/main/java/com/jerboa/Utils.kt Outdated Show resolved Hide resolved
@beatgammit beatgammit force-pushed the feat/fix-crash-on-community-url branch from b287be8 to c20ef61 Compare June 14, 2023 04:00
@beatgammit
Copy link
Contributor Author

beatgammit commented Jun 14, 2023

I redid the PR and added some tests. The code is kind of repetitive, but hopefully easy to follow.

I'm interested in adding a bunch of tests, so let me know if there's a good reason not to upgrade to JUnit 5, since I may want to do that to get parameterized tests working properly.

@beatgammit beatgammit force-pushed the feat/fix-crash-on-community-url branch 2 times, most recently from fcb80a4 to 32b56fd Compare June 14, 2023 04:09
@beatgammit beatgammit requested a review from dessalines June 14, 2023 04:18
@beatgammit beatgammit force-pushed the feat/fix-crash-on-community-url branch from 32b56fd to 117c58f Compare June 14, 2023 07:16
@helpimnotdrowning
Copy link

helpimnotdrowning commented Jun 14, 2023

this doesn't seem to cover reddit-style* links like c/[email protected] or u/[email protected] (missing 1st slash), is this intentional?

(ok I guess these are all technically "reddit-style" but i think my point gets across)

@beatgammit
Copy link
Contributor Author

beatgammit commented Jun 14, 2023

Links without the leading / are incorrect on Reddit IIRC (at least on old Reddit), and I personally think they're bad style, so I tried to keep this PR uncontroversial. Also, Jerboa doesn't handle user links anyway, so I figured I'd revisit once it does.

It intend to figure out how to do a plugin for the markdown library to get it to process links, and I suppose I can look into that when I do.

@beatgammit beatgammit force-pushed the feat/fix-crash-on-community-url branch from 117c58f to 8b75fd2 Compare June 15, 2023 14:09
@beatgammit beatgammit force-pushed the feat/fix-crash-on-community-url branch from 8b75fd2 to 351e981 Compare June 15, 2023 22:17
@beatgammit beatgammit force-pushed the feat/fix-crash-on-community-url branch from 351e981 to 1076794 Compare June 15, 2023 22:27
This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.
@beatgammit beatgammit force-pushed the feat/fix-crash-on-community-url branch from 1076794 to 7bc1030 Compare June 15, 2023 22:36
@beatgammit beatgammit force-pushed the feat/fix-crash-on-community-url branch from 50b96c0 to 92abc20 Compare June 15, 2023 23:42
@twizmwazin twizmwazin merged commit dcaf991 into LemmyNet:main Jun 15, 2023
Chris-Kropp pushed a commit to Chris-Kropp/jerboa that referenced this pull request Jun 16, 2023
* Handle more URL types

This is a partial fix to LemmyNet#556, but it at least prevents crashing when an unknown
URL type is encountered. There is still some work to do, since the
markdown plugin doesn't properly interpret URLs the way we want.

User-related parsing still doesn't work because there's work needed in
the markdown layer, but once that's done, this should just work as
intended.

* Support http links
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants