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

Add opt to mark as read when opening link #113

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

antznin
Copy link

@antznin antznin commented May 12, 2024

Hi,

This PR adds the argument --mark-read-on-open to mark the items as read (write it in history) when opening a link.

Curious to have comments on this. On my side, it felt missing that the article was marked unread even though I did go through the article in my browser (not the comments, though).

Add the argument `--mark-read-on-open` to mark the items as read (write
it in history) when opening a link.

Signed-off-by: Antonin Godard <antoningodard@pm.me>
@bensadeh
Copy link
Owner

Hey @antznin and thanks for the PR! I think this behavior makes sense for sure.

There is currently one issue with the way I implemented the mark as read feature as it currently stands: it is tightly coupled with the feature where we mark new comments as read.

For example, if we mark the story as read, we both mark the story as read and add a timestamp to when the story was read. This means that a user who has opened a link and later want to read the comment section will have some comments marked as "new" even though the user hasn't read the comment section at all.

Maybe it isn't an issue at all (as it could be interesting to see "new" comments). I am leaning towards having opening an article in any capacity (reader mode, open link in browser, comment section) mark the story as read. What do you think?

@antznin
Copy link
Author

antznin commented May 14, 2024

Hi @bensadeh,

I'm not sure I see the same behavior, or that I understand what you mean correctly. When passing --mark-read-on-open, if I open a link without opening the comments (with o) and go back to circumflex, the title is in italic and marked as read (that's what we want). Then, if I open the comment section for that same article the comments are not marked as new for me. Did you expect all the comments to be marked as new (with )?


To go further on the different modes when --mark-read-on-open is passed:

  • When opening an article in reader mode (Space), it is not marked as read when returning to the main menu.
  • When opening the comments in the browser (c), it is not marked as read when returning to the main menu.

So this change is just limited to keymap o if that makes sense.

I think that's what you're referring to: should we consider going beyond my change and have the possibilty to mark as read/mark as read by default for these two other keymaps (Space and c)?

I think it makes sense, yes. Having flexibility on what is configurable would be nice, but might add too many options.

@bensadeh
Copy link
Owner

Just to clarify with an example:

  • At 10:00 a clx user opens an article with space or c
  • circumflex internally marks that the article is read at 10:00
  • A hacker news user comments on the same story at 10:01
  • The same clx user from before opens the comment section at 10:02
  • circumflex will mark the comment from 10:01 as new since it is newer than 10:00 even though this is the first time the clx user reads the comment section

However, I this might not be an issue in practice. I like your suggested change anyway.

Would you be able to update the PR to have this be the default behavior (i.e. no config)? Don't worry about space or c for now, just have it apply for opening the link using o like in your original PR.

@antznin
Copy link
Author

antznin commented May 28, 2024

Hi @bensadeh,

Thanks for your comment!

I will get back to this in a couple of weeks, as I don't have access to my computer atm (vacationing 🌴)

@bensadeh
Copy link
Owner

Enjoy your vacation, my friend 😎

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.

None yet

2 participants