-
Notifications
You must be signed in to change notification settings - Fork 37
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
Am I interacting with this feed? #254
Comments
Rough order of things, based on what blocks what:
|
I don't know what other combinations mean, but this is enough to show "not read, but I don't care" == read and not important and important_changed is not None. Using a new flag or an entry tag makes things more complicated, and there's no other meaning to associate with "marked as not important by hand, not by default" anyway (I think). |
Regarding how we store timestamps: We could do it with entry metadata (#253), but that's not implemented yet, and using it in queries would be a pain. It makes more sense to do the simplest thing possible now, and reconsider re-unification later. The "simplest possible thing" seems to be to have another Entry attribute (and table column), so we'll do that. |
The minimal API: # Reader
mark_as_read(...) # use the real "now", backwards compatible
mark_as_read(..., now: Optional[datetime]) # use a custom "now"; mainly for plug-ins
# TODO: find a better argument name than now... timestamp? last_modified?
class Entry:
...
read_last_modified: Optional[datetime]
# Storage
mark_as_read_unread(..., now: Optional[datetime])
# idem for important Notes:
|
To do (for storing stuff):
|
We're mostly done with the "store modified" part. The UX for "don't care" needs a bit of work, though... In 4278939 (and 59004b3) I had to make the "unimportant"/"unread" buttons button set important_modified=None, because otherwise making a read, important entry not important makes it "don't care", which is not what we want (not all the time, anyway – sometimes you just want to undo a "mark as important"). ... this tri-state of important (true, false and modified, false and not modified) is a bit confusing (or at least, the way we use it to infer another flag is) ... I should likely make a diagram to make more sense of it. |
Spent about 12 hours on this until now. |
We have conflicting interests, so it's likely time to address #254 (comment):
Currently, "don't care" == read and not important and important_modified is not None. Here are all the possible combinations and their meanings:
Importantly, the interpretation is done outside of Reader, in the web app; because of this, there are two places where the web app needs to do extra handling:
The web app allows the following transitions (source): The conflicting interests are as follows:
On top of this, if the "don't care" logic implemented in the web app is useful, it should end up in Reader (we can probably repurpose the mark_entry_as_... methods for this, as part of #291). But, if it becomes stable, it should be easy to explain. |
The current state as described above impossible to satisfy all the interests. Adding a third flag for "don't care" would even complicate things further. Even worse, its meaning overlaps with unimportant with important_modified set. But... We can translate the interests in terms of actual requirements:
Proposal 1: unimportant with modified == don't careWe can express user "don't care" as just unimportant with important_modified set. This is simple to explain ("user explicitly set unimportant"), and is not coupled to read in any way.
For plugins, we can free up read-none by backfilling pre-#254 read to entry.added; for consistency, we should do the same for important. This can also be explained more easily ("plugin explicitly set as read"), and is not coupled to important in any way. (The unimportant-with-modified entries already marked by The full table becomes:
The UI logic becomes (a bit) simpler too, the tri-state involves only important:
This doesn't really clear things up from a Reader perspective, though. |
Proposal 2: important: bool|NoneChange important to be bool|None. Users set Pros:
Cons:
Backwards-compatible proposal for get_entries(important=...):
The last two aren't required, but it's still a good idea to find names for them. Update: Here's a full typed proposal for the get_entries() API – instead of mixing bool|None and string literals, we have a set of literals, and map bools and None to some of the values (note BoolFilter is subset of OptionalBoolFilter): BoolFilter = Literal[
'true', # value is True (equivalent to filter=True)
'false', # value is False (equivalent to filter=False)
'all', # equivalent to filter=None
'default', # equivalent to filter='all'
]
OptionalBoolFilter = Literal[
'true', # value is True (equivalent to filter=True)
'false', # value is False
'unset', # value is None
'not true', # equivalent to filter=False
'not false',
'all', # equivalent to filter=None
'default', # equivalent to filter='all'
]
def get_entries(
read: BoolFilter | bool | None = None,
important: OptionalBoolFilter | bool | None = None,
) -> None: ... Of course, we don't have to go with the full thing; initially, we can go with the minimum needed, and add more values later: OptionalBoolFilter = Literal[
'true', # ~= True
'false',
'unset',
'not true', # ~= False
'all', # ~= None
]
def get_entries(
read: bool | None = None,
important: OptionalBoolFilter | bool | None = None,
) -> None: ... We could also model BoolFilter and OptionalBoolFilter as enums, but:
Update: EntryCounts should also likely be updated to count "important == None". Update: The UI logic (note "unimportant" becomes "clear important" so it's harder to confuse with "don't care"): if not important: # False, None
important_button() # -> True
if important is not None: # True, False
clear_important_button() # -> None
if important is not False: # True, None
dont_care_button() # -> False |
Going with proposal 2. To do:
|
Also, set_entry_read/important() should take exactly True, False, None (instead of anything you can call bool() on). Also, update the mark_as_read plugin to explicitly set important=False, and read_modified and important_modified None. For #254.
After a lot of deliberation, here's a naming scheme for get_entries(important: bool|None|TristateFilter):
Requirements:
Notes:
|
Add versionchanged/versionadded. For #254.
A quick note on what additional stats should look like. While Furthermore, there are additional questions For maximum flexibility, we could instead provide a dataframe-like collection containing the values of all interesting fields for each entry (published, updated, added, read, read_modified, ...). |
https://gist.github.com/lemon24/93222ef4bc4a775092b56546a6e6cd0f
|
I'd like to know if I'm interacting with a feed, i.e.:
Open issues:
entry_dedupe
needs to be able to copy/change any timestamps we store.The text was updated successfully, but these errors were encountered: