Remove LinkSpan in favor of URLSpan

[saket]

Hey @noties, thanks for open sourcing this project. You have saved me a lot of time. Small suggestion about links. I think that the existence of LinkSpan is not justified in Markwon. It requires extra effort to support it and breaks compatibility with other libraries.

Replacing it with URLSpan will enable,

• Android to pick-up textColorLink from TextView's theme instead of picking it from the Context's theme. This is a problem if I use Application context for supplying a SpannableConfiguration object through my Dagger graph.

• Third-party libraries like my Better-Link-Movement-Method and perhaps others that work with URLSpan for extracting links will continue to work just fine. My library will treat the text as the url if ClickableSpan was found instead of an URLSpan.

The responsibility of resolving links is not on a markdown processing library. This could be optional, but the responsibility of handling links should be left to MovementMethod.

In case you're wondering why can't I just register a LinkResolver to fix this problem, I extract the location of a URLSpan using a custom MovementMethod that also keeps a track of TouchEvents. This let's me display contextual popups where the links are present.

Thoughts?

[saket]

On a second thought, offering a global LinkSpan.Resolver that can work on all TextViews is a nice idea, but I think it should atleast be made optional. :)

I could send a PR if you want.

[saket]

Second update: oh wow, I just realized that both these problems can be solved by simply making LinkSpan extend URLSpan and removing the overriden #updateDrawState() method.

[noties]

Hey @Saketme !

Thanks for kind words and interest in the project! Please tell me (after all the updates 😉 ) if extending URLSpan instead of ClickableSpan would suffice? Because it's an easy thing to do whilst keeping backwards compatibility (I assume, you are OK with current LinkResolver?).

I also find that using TextPaint's linkColor makes more sense here. I will remove initialized value from default configuration, so if it's not specified whilst configuring - spans would use linkColor automatically.

[saket]

Hahaha, you can see my third update here: https://github.com/Saketme/Markwon/commits/saket.linkspan

I made LinkSpan extend URLSpan and so far it seems to be working fine. I agree that this is the better solution without breaking existing clients.

The current LinkResovler will also continue to work. The only difference will be that it'll only get called for spans that the TextView's MovementMethod does not handle. Does this make sense? I could send a PR.

[noties]

@Saketme hey! Sorry for late reply.

I have just created a new branch for the next 1.0.5 release and pushed there discussed changes. You can find it here.

I would like if you could check how it works for your use-case 🙌

The solution is a bit different. If SpannableTheme has linkColor set it will be used as previously. I have changed default configuration to omit configuring linkColor. So if it was not set explicitly a default link color (of a TextView or any holder of rendered markdown) will be used.

[saket]

No problem. I just tested the commit and it seems to be working perfectly. Thank you :)