Support selecting multiple ranges

[karlicoss]

I found one bug though. If I make several text selections on one page, press Ctrl-Alt-Y to capture them, the selections end up concatenated without a newline character.

If it's not too heavy duty, it would make sense to me to create a

#+BEGIN_QUOTE #+END_QUOTE

block for every selection. I don't think interleaving my comments with selections/quotes is necessary, but I think it makes sense to save selections as quotes.

Can I do this with a different org-mode capture template or does it require changing the extension?

Originally posted by @petr-tik in https://github.com/karlicoss/grasp/issues/7#issuecomment-565823942

[karlicoss]

Good catch @petr-tik ! I've totally forgotten about it, so it's gonna require both changes in extension to support getRangeAt and also changes in the backend script to split selection parts. The backend bit is trickly one, need to think what would be a good way to specify such a template.

[petr-tik]

Judging from this code in the extension,

https://github.com/karlicoss/grasp/blob/master/src/js/background.js#L90-L100

you call toString() on the selection, which bundles them together.

If I understand it correctly, we can remove the toString() call, iterate over all selections and wrap them inside #+BEGIN_QUOTE #+END_QUOTE blocks.

Do we need any changes in the server though? It looks like we can just send a bigger selection string with quote blocks inside.

[karlicoss]

Yep, you're right about iterating, but extension doesn't know anything about BEGIN_QUOTE/END_QUOTE; this is handled by backend based on the template. (e.g. I don't use begin_quote/end_quote).

[petr-tik]

Would you prefer not to use begin/end quotes or add support to a custom template?

[karlicoss]

Discussed with @petr-tik and it seems that it'd be reasonable to pass multiple selections (as an array) from the extension and accept proper f-string as a template; that way it'd be easy to format the list to user's liking.

[karlicoss]

Actually, f-strings are syntax, so wouldn't be possible to pass them as a commandline argument.

But this brings us closer to supporting multiple seleciton..

[petr-tik]

since you merged the PR, is there anything else that needs to be done for multiple selections apart from updating client-side javascript to pass the full selections vector?

I guess the server should offer a graceful fallback, if the template doesn't specifiy how to deal with multiple selections by gluing them together into 1 block (for example)

[karlicoss]

Yep! Just saying it should be easier now.

Extension changes:

• change the type of selection and use getRange here

Backend changes:

• change the signatures of as_org and make Config/DefaultConfig join the selected strings by default

---

The tricky bit is: the extension will be updated automatically; whereas the backend requires a manual intervention. That unfortunately means that if we start sending multiple selections to the backend, it will break it for the existing users :( So there are three options here:

1. If the user selected a single range, send it as a string (this will still break for people who did multiselection). Very easy, but half-assed.
2. Make it defensive: try sending as a list, and if it failed, do it the old way. Easy, reliable, but sort of meh.
3. Make it configurable: send it as a single string for the old addon users + add an option to switch to sending a list. For the new installs, always send as a list. Requires more settings etc, but the most robust way.

Note to self: in the future, perhaps always need to expose a backend version throught an endpoint. So the frontend can act accordingly...

Ugh. How hard can it be to to write a clipping extension :)

[petr-tik]

Make it defensive: try sending as a list, and if it failed, do it the old way. Easy, reliable, but sort of meh.

If we can confirm that the failure was due to format discrepancy and nothing else, trying new way and retrying with the old format on failure should work, no?

Why "meh"?

Make it configurable: send it as a single string for the old addon users + add an option to switch to sending a list. For the new installs, always send as a list. Requires more settings etc, but the most robust way.

Note to self: in the future, perhaps always need to expose a backend version throught an endpoint. So the frontend can act accordingly...

We can start with that now - who knows what other features you will extend grasp with in the future. Adding a "/version" endpoint to backend and client extension is purely additive. What happens if a new client sends a GET to the non-existing "/version" on the old backend. I tried googling about python's stdlib http.server and couldn't find anything. If the error doesn't crash the server and returns a consistent error code, we should be able to use that error code as an indicator of an old version eg.

enum Version {
Prehistoric, // introduced when the backend still didn't have a /version endpoint
Tracked(u64), // returned by the backend with a version endpoint 
// incremented when new features are added or breaking changes are introduced
}

excuse the rust snippet - enums are too nice