Be smarter about moving to the next entry after an action

[groks]

Add an extension point elfeed-search-next-entry and call it instead of the hard-coded forward-line after performing an action such as tagging/untagging.

Instead of always moving forward, move back if it looks like you're reversing up the search results.

Maybe there's a fancier algorithm?

[skeeto]

Interesting, I hadn't expected extension here. I don't object to the
idea of putting some kind of configuration here (defcustom, etc.), but I
don't want to change the way it currently operates. In particular, the
more complex elfeed-search-next-entry added in 941b077 won't advance
past the first entry if it's read and the second is unread. By default
I'd like to stick to the very obvious and unsurprising current behavior.

As a side note, outside of certain compiler bugs, running the test suite
("make test") should come out totally clean. After the second commit,
the compiler complains about the use of next-line and previous-line (for
interactive use only).

[porglezomp]

I agree with the notion that the defaults should be obvious, but I'd love to have the option to enable this since I very often read a given day's feeds from bottom to top.

[agzam]

This would be useful for me as well, most of the time I read things from the bottom, sometimes marking them as 'read', and it mildly annoyed me to have to manually move the cursor each time.

I fooled around, modifying elfeed-search-untag-all, and the initial draft came up as something like this:

(defun elfeed-search-untag-all (tag)
  "Remove TAG from all selected entries."
  (interactive (list (intern (read-from-minibuffer "Tag: "))))
  (let ((entries (elfeed-search-selected)))
    (elfeed-untag entries tag)
    (mapc #'elfeed-search-update-entry entries)
    (unless elfeed-search-remain-on-entry
      (if (when-let ((nxt-entry
                      (save-mark-and-excursion
                        (when (use-region-p)
                          (goto-char (region-end)))
                        (forward-line)
                        (car (elfeed-search-selected)))))
            (member 'unread (elfeed-entry-tags nxt-entry)))
          (forward-line)
        (progn
          (when (use-region-p) (goto-char (region-beginning)))
          (forward-line -1))))))

And then I found this PR, so ended up simply advising the function:

(defun elfeed-search-untag-all-a (_)
  "Advice to the previous elfeed-entry unless the next one is unread."
  (when-let ((nxt-entry (car (elfeed-search-selected))))
    (unless (member 'unread (elfeed-entry-tags nxt-entry))
      (forward-line -2))))

(advice-add 'elfeed-search-untag-all :after 'elfeed-search-untag-all-a)

I'm not suggesting anything, just posting it here, maybe someone finds it useful.

[yantar92]

I agree that this patch, if accepted, should be shielded behind defcustom: move next entry, previous entry, next unread, previous unread. Although, I feel that moving to unerad may lead to unexpected results if the use case involves marking items read selectively (not one by one from top to bottom or from bottom to top). So, maybe the direction needs to be customizeable.

There are also at least two users who expressed interest, so probably it is worth merging.
I can work on patch amendments if other maintainers agree.

[yantar92]

Also, there is elfeed-search-remain-on-entry added after this patch was submitted. So, we have a precedent.

[minad]

@yantar92 Honestly I prefer the setting elfeed-search-remain-on-entry. For most operations it does not really make sense in my opinion to move to the next entry (for exampling yanking). Of course for showing the entry ("RET"), moving to the next entry is somewhat useful. But then, if you jump to the show buffer, you can simply press "n" there, to continue. Therefore I am not sure if we really need this patch, and I would hold it back for a bit longer. If we decide to implement a smarter move, I would introduce a more generalized variable elfeed-next-entry-function and offer some options there.

[minad]

Alternatively one could consider to install a local post command hook in the search buffer, and then parametrize it for only a few commands, where movement makes sense. The current hard coded behavior is not great.

[yantar92]

Daniel Mendler ***@***.***> writes:

@yantar92 Honestly I prefer the setting `elfeed-search-remain-on-entry`. For most operations it does not really make sense in my opinion to move to the next entry (for exampling yanking). Of course for showing the entry ("RET"), moving to the next entry is somewhat useful. But then, if you jump to the show buffer, you can simply press "n" there, to continue. Therefore I am not sure if we really need this patch, and I would hold it back for a bit longer. If we decide to implement a smarter move, I would introduce a more generalized variable elfeed-next-entry-function and offer some options there.

+1 about generalized variable. Maybe even something like `((yank . move-function-for-yank) (read . move-function-for-read) ...)`. Akin `org-fold-show-context-detail`. There is indeed no rush. Not after this being on hold for years.

…

-- Ihor Radchenko // yantar92, Org mode maintainer, Learn more about Org mode at <https://orgmode.org/>. Support Org development at <https://liberapay.com/org-mode>, or support my work at <https://liberapay.com/yantar92>

[minad]

Maybe even something like ((yank . move-function-for-yank) (read . move-function-for-read) ...). Akin org-fold-show-context-detail.

Indeed, this is what I had in mind. I think this variable should then also supersede elfeed-search-remain-on-entry.

[minad]

I added a simple generalization of elfeed-search-remain-on-entry.