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

Generate RSS feed at /rss.xml #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Generate RSS feed at /rss.xml #8

wants to merge 2 commits into from

Conversation

frnsys
Copy link
Owner

@frnsys frnsys commented Dec 29, 2020

@breezykermo I kind of prefer the JSONL format since it's more compatible with some other tools I have, but exposing highlights as RSS is useful. This PR keeps JSONL as the main storage format but provides a feed at /rss.xml. Would we be able to combine our changes in some way?

@breezykermo
Copy link
Contributor

definitely. what does your JSON object look like on disk at the moment? This looks good to me, the other three changes/additions that I can think of in my version at the moment are:

  1. An added note field in addition to quote
  2. Extra clients for iOS, DevonTHINK and Python
  3. Dockerization at the outer layer.

If you don’t have any problem with adding these, I can synthesize them as a PR from my version to here.

@breezykermo
Copy link
Contributor

Just had a play around with your version. I think we need to disambiguate fields a bit. My clips currently look like this

{
  "quote": "The text that is highlighted at time of clip",
  "note": "The user-generated note that is typed at time of clip",
  "time": 12321312311,
  "tags": ["test", "example"],
  "href": "https://baseurl.com/that-the-clip-comes-from"
}

The server you have expects at least additional "text", "html" and "title" attributes on clips.

In my version of a clip, it's not necessarily sourced from a webpage (it could also be a DT document), and as such I think my "quote" is analogous to your "html". I don't have strong opinions about what we call this, and it can present the content in the same way that your hili currently presents as HTML (as plain text from DT is also technically valid HTML). Your version also has a "text": how is this distinct from "quote"/"html"?

"title" is an attribute that I like from your version that isn't in mine, if I'm correct in assuming that it refers to the title of the document from which the clip is sourced. This is not to be confused with what content should be put in the <title/> tag in the XML feed, which from my perspective should be the quote/html I refer to in the para above (the quoted content that anchors the clip). This is important so that 100 clips from the same document/URL show as distinct quotes, rather than 100 replications of the document's title.

I think our treatment of "note" is the same. In the server we could support this as HTML like quote/html, so that a more complex could produce styled notes.

@breezykermo
Copy link
Contributor

I also like how your server groups clips according to matching hrefs when presenting them in HTML. One thing to note here is that, for grouping clips from DT urls, it's important that the groups are created based on the URL stripped of query parameters. This is because DT allows you to open directly to page numbers using URLs like x-devonthink-item://936945E8-5596-4B93-B58C-E3DF55E230AF?page=15.

@breezykermo
Copy link
Contributor

One final point is that I excised file storage from my version of hili, as I primarily use it for text. But given that "file" seems to be optional in your clips, it doesn't bother me at all to have that as an attribute in clips; I might sophisticate the DT client later to allow sending the image as base64 like your firefox client does.

@breezykermo
Copy link
Contributor

@frnsys most of these adjacent comments now addressed in #10, so from my end this is good to merge! Storing as JSONL and generating XML dynamically works for me.

@breezykermo breezykermo mentioned this pull request May 1, 2021
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.

2 participants