org-appear breaks org-fill-paragraph

[DarwinAwardWinner]

When I press M-q to fill the current paragraph, it gets filled according to the currently visible text. This results in potentially very different fills depending on where the point is in the paragraph. Here's an example:

* Bad fill
This paragraph gets filled very differently when [[https://github.com/awth13/org-appear][org-appear]] is used
and point is inside the link.

Put that in an org-mode buffer and turn on org-appear mode. Put the point at the start of the paragraph and press M-q (org-fill-paragraph). Then move the point inside the link and press M-q again. On each press of M-q, filling happens as if the currently shown text were the actual content of the buffer. Ideally, org-fill-paragraph should fill the same way regardless of the point's location.

[DarwinAwardWinner]

The following advice seems to resolve the issue, by ensuring that org-appear is "disabled" while org-fill-paragraph runs.

(define-advice org-fill-paragraph (:before (&rest args) fix-org-appear)
  "Fill consistently regardless of org-appear element status."
  (ignore-errors
    (when org-appear-mode
      (let ((current-elem (org-appear--current-elem)))
        (when current-elem
      (org-appear--hide-invisible current-elem))))))

[awth13]

Thank you for posting the issue, @DarwinAwardWinner!

The fill code (fill-region-as-paragraph) cuts lines based on columns of text rather than buffer positions -- which totally makes sense -- and columns are moved, both visually and in code, by hiding/unhiding Org elements. I would really like to avoid using define-advice but, after some investigation, I don't see any other solution for this. I will test your advice code and use it in patching the issue if that's OK. You are welcome to provide it as a PR, of course.

[DarwinAwardWinner]

Note that I'm not necessarily suggesting the above advice be adopted verbatim. I'm not 100% certain that org-fill-paragraph is the optimal place to put the advice, and for inclusion into the package the advice should probably be made more robust (e.g. an error in the advice should be demoted to a warning in order to let the original function still run without error). Regardless, feel free to use this as the basis for a fix.

[DarwinAwardWinner]

Hmm, it seems that there are other commands that need a similar treatment, such as whatever C-c C-c does inside tables. I've been seeing it throw errors, which unfortunately I can't debug directly thanks to the special treatment of errors in post-command-hook. I'll try to drill down on the issue when I get the chance.

Maybe a more general solution is to put the above code into pre-command-hook or something.

[DarwinAwardWinner]

For what it's worth, here's what I have so far:

;; Don't allow this function to throw errors, because that's a no-no
;; in `post-command-hook'.
(define-advice org-appear--post-cmd (:around (orig-fun &rest args) demote-errors)
  (with-demoted-errors "Error during `org-appear--post-cmd': %S"
    (apply orig-fun args)))
;; Hide things that org-appear has made visible before running certain
;; functions that rely on those things being hidden.
(defun org-appear--hide-current-elem ()
  (ignore-errors
    (when org-appear-mode
      (let ((current-elem (org-appear--current-elem)))
        (when current-elem
          (org-appear--hide-invisible current-elem))))))
(define-advice org-appear-mode (:after (&rest args) set-pre-command-hook)
  (if org-appear-mode
      (add-hook 'pre-command-hook #'org-appear--hide-current-elem nil t)
    (remove-hook 'pre-command-hook #'org-appear--hide-current-elem t)))

Both of these advice definitions can of course be integrated into their respective functions in the package. This seems to work well so far with no noticeable slowdowns (unlike some previous solutions I tried).

[DarwinAwardWinner]

I will note, with the above code, sometimes org-appear fails to make things appear after e.g. M-q. I suspect this is probably caused by #21.

[awth13]

The issue should be fixed now. I added a function in pre-command-hook that detects if the next command is going to modify the buffer and hides the current element. Thank you for your suggestions, @DarwinAwardWinner!

The pre-cmd function currently fires on org-fill-paragraph and org-ctrl-c-ctrl-c. If there are similar issues with any other commands, please let me know.

[awth13]

This issue and #21 seem to be resolved now so I am closing them.

Thank you for your input and let me know if similar behaviour affects any other commands!