>> + (mapcar #'org-invisible-p >> + (number-sequence (line-beginning-position) >> + (1- (line-end-position))))) > > This is a bad idea. org--line-visible-p will be called for every single > invisible headline. If you check every single point at every single > invisible headline, it can be extremely slow.
Hm, of course it would only check invisible headlines of the same level, thanks to the logic of the navigation command. However, I do see the concern. > Better do something like below (or maybe even without narrow-to-region, > not sure if that may cause significant overhead): > > (defun org--line-visible-p () > "Return t if the current line is partially visible." > (save-restriction > (narrow-to-region (line-beginning-position) (1- (line-end-position))) > (let ((visible t) > (p (point-min))) > (while (and visible (< p (point-max))) > (when (org-invisible-p p) > (setq visible nil)) > (setq p (next-single-char-property-change p 'invisible))) > visible))) The issue with that is that it reintroduces the thing the patch is trying to fix, as it considers any partially invisible line as fully invisible (while the opposite should be the case). I also don't see much of a difference when profiling, unless I severely screwed up. I just manually prepared a buffer with 10k invisible headings (each 83 chars long) and plugged a visible one of the same level above and below the huge invisible block. C-c C-f does take a second plus change in that case for both cases. The main issue is that a hot-circuit approach would only ever optimize navigating across many visible blocks, if we want a partially visible heading to be treated like a visible one (instead of an invisible one like it is at the moment). However, we could use a similar hack as the original implementation and only check a couple of characters, if performance really is a concern there. In that case, I think it would be best to check the *last* few characters (as it makes more sense that the stars at the beginning are hidden than the actual title at the end). That would mean to replace the original implementation with something like (defun org--line-visible-p () "Return t if the current line is partially visible." (let ((min-pos (max (line-beginning-position) (- (line-end-position) 3))) (max-pos (1- (line-end-position)))) (and (memq nil (mapcar #'org-invisible-p (number-sequence min-pos max-pos))) t))) which basically removes any relevant slowdown. But I'd really only recommend going that far if necessary, given it adds a random magic number. Thoughts? Cheers, D.