jsonMartin / readwise-mirror

MIT License
52 stars 11 forks source link

Bug: Readwise Mirror generates a locationUrl even for non-books #34

Closed tdnzr closed 11 months ago

tdnzr commented 11 months ago

Problem

main.ts sets the variable locationUrl as follows:

const locationUrl = "https://readwise.io/to_kindle?action=open&asin=${book['asin']}&location=${location}";

This is fine if the highlight corresponds to a book, but meaningless otherwise. IIRC in that case the URL looks like this:

const locationUrl = "https://readwise.io/to_kindle?action=open&asin=null&location=${location}";

Solution

For non-valid locationUrls (i.e. if book or book['asin'] are null), set locationUrl to null.

(Note that I can't make a pull request for this, because I don't know how to write the corresponding TypeScript code.)

Side Benefit

If this fix was implemented, one could also optionally adjust the highlight template in the readme from this:

{%- if category == 'books' %} ([{{ location }}]({{ location_url }})){%- endif %}

To this:

{%- if location_url} ([{{ location }}]({{ location_url }})){%- endif %}

The second version looks a bit simpler to me.

Though there's also value in having at least one example in the readme of how to check whether a field has a specific non-empty value (as in if category == 'books').

jsonMartin commented 11 months ago

Hi @tdnzr , thanks for the suggestion but since this isn't causing any run-time issues, I don't personally plan on adjusting this one. It shouldn't be an issue if the location_url isn't used when not a book type. I'd welcome a pull request changing this however.