Ensure that isearch can see through overlays added by magit-section

[cpitclaudel]

Thanks a lot for all your hard work on magit.

I'd like to be able to use isearch in magit-status buffers. This feature would be useful when splitting a large change into multiple commits, to ensure that I haven't forgotten to commit part of a change.

For example, in the course of working on a feature, I might rename an identifier throughout my project. When I'm done with the rename and the change, I'll use selective staging to create a commit including only the rename. To make sure I include all lines affected by the rename, I currently press TAB on all files in the "Unstaged changes" section, and then use isearch to look for the renamed symbol. If I don't reveal the diffs for all files, isearch can't search through them, it seems.

Based on https://www.gnu.org/software/emacs/manual/html_node/elisp/Invisible-Text.html and looking at how org-mode handles this case (org-mode reveals hidden text when it contains an isearch match), I think the right implementation would be to set a non-nil isearch-open-invisible property on the overlays used to hide diffs in magit-status.

I have done this in this PR.

[cpitclaudel]

One thing I haven't figured out, however, is how to make the reveal persistent. After I complete a search that revealed a section, that section is immediately hidden again.

[tarsius]

Problems:

1. Expanded section collapses again after finishing search, as you mentioned.
2. Some sections that haven't been expanded since last being recreated have not been "painted" yet. Diffs for example are not painted until expanded for performance reasons.
3. The bodies of some sections are not even inserted until they are expanded. Again for performance reasons.

There's nothing we can do about (3). To address (1) and (2) isearch would have to be taught to call magit-section-show when appropriate. Maybe isearch has a hook for that, but I doubt it. So you would probably have to find the place where isearch considers isearch-open-invisible and then advice/redefine the respective function to do so.

I am not gonna do that myself any time soon and if I do, then I would do so for swiper instead because that's what I use.

[tarsius]

Would you like me to merge this as-is?

[cpitclaudel]

Probably not; 2 should be fixed. I don't want to merge a partly broken feature in.

To address (1) and (2) isearch would have to be taught to call magit-section-show when appropriate. Maybe isearch has a hook for that, but I doubt it.

It's easy to do that, actually:

(defun magit-section-open-invisible (overlay hide)
  "Show or HIDE the section corresponding to OVERLAY."
  (when-let ((section (overlay-get overlay 'magit-section)))
    (if hide (magit-section-hide section) (magit-section-show section))))

… 
(overlay-put o 'magit-section section)
(overlay-put o 'isearch-open-invisible-temporary #'magit-section-open-invisible)

But the issue is that magit-show deletes the overlay instead of just toggling it, which confuses isearch when it tries to re-hide it.

[tarsius]

I've just pushed my implementation.

For Isearch this should work already. Swiper users have to wait until abo-abo/swiper#2689 is merged.

[cpitclaudel]

👏 👍 🙇

[basil-conto]

Swiper users have to wait until abo-abo/swiper#2689 is merged.

Done; I think three weeks was long enough.