Emacs 29: org-fold support #563
Comments
|
Yes. I was afraid that this won't work anymore. We will wait until this feature has been released with a new Org version. I think you can work around the issue in the meantime by using the old overlay-based folding mechanism. PR welcome which adds support for the new property based technique. I wonder how Isearch handles this. We could ask on the Org mailing list since there is currently some discussion about breakages caused by the org-fold merge. |
|
I looked at the org-fold merge and it seems difficult to support this, since org-fold hacks into Isearch to convert text-property-based folding into overlays on the fly when there are Isearch matches, such that the usual Isearch unfolding keeps working. This means we either have to ask Org to also temporarily switch partially to overlays on jump or during the search or we have to ask Org to manually do the unfolding for us, which would be more efficient. I think for the near future one should use Lines 1248 to 1272 in df68639 |
|
I see, thanks for detailed reply and checking out org-fold merging investigation. I would take your suggestion to temporarily use |
|
For reference, see also yantar92/org#42 and awth13/org-appear#5. It seems org-fold provides |
|
@minad Thanks for noting me up. I will keep watching this discussion and workaround solution.
Can you provide a temporary patched workaround version? That will be great. |
|
@stardiviner In awth13/org-appear#5 (comment) @yantar92 told me that there is no official API (yet?) for temporary unfolding and restoration, so we should wait until one is added. |
|
I see. Thanks. |
|
It was suggested that the discussion should be moved to the ML but I don't have time currently to pursue this. If you are interested go ahead. |
|
I got a response from org-fold developer Ihor's API. (defun test (pos)
(interactive "nEnter point to be revealed: ")
(read-char "Revealing point temporarily. Press any key...")
(save-excursion
(org-fold-save-outline-visibility nil
(goto-char pos)
(org-fold-show-set-visibility 'local)
(read-char "About to restore the previous fold state. Press any key...")))
(read-char "Now, reveal the point permanently. Press any key...")
(goto-char pos)
(org-fold-show-set-visibility 'local))I tested and confirmed it works fine. |
|
By the way, I propose an suggestion which not related to this issue. I think For example: If without " (defun consult-org-heading (&optional match scope)
"Jump to an Org heading.
MATCH and SCOPE are as in `org-map-entries' and determine which
entries are offered. By default, all entries of the current
buffer are offered."
(interactive (unless (derived-mode-p 'org-mode)
(user-error "Must be called from an Org buffer")))
(let ((prefix (not (memq scope '(nil tree region region-start-level file)))))
(consult--read
(consult--with-increased-gc (consult-org--headings prefix match scope))
:prompt "Go to heading: "
:category 'consult-org-heading
:sort nil
:require-match t
:history '(:input consult-org--history)
:narrow (consult-org--narrow)
:state (consult--jump-state)
:group
(when prefix
(lambda (cand transform)
(let ((name (buffer-name
(marker-buffer
(get-text-property 0 'consult--candidate cand)))))
(if transform (substring cand (1+ (length name))) name))))
:lookup #'consult--lookup-candidate
+ :initial "*")))Although user can override function code by themself, but I think this default behavior is best. WDYT? |
|
Thanks for the example. Can you turn this into a PR to the aforementioned consult--invisible- functions please? Regarding initial input - we won't add this by default but you can easily configure initial input via consult-customize as documented in the README. Instead of doing that I suggest you use a better completion style configuration, e.g., take a look at orderless. |
Ok, I will propose a PR.
Oh, I used to read the README, but not really understand how |
|
Here is the PR prototype, is this ok? ;; PATCH: a temporary workarund for `consult-org-heading' with new `org-fold'.
(defun consult--invisible-open-permanently ()
"Open overlays which hide the current line.
See `isearch-open-necessary-overlays' and `isearch-open-overlay-temporary'."
(dolist (ov (let ((inhibit-field-text-motion t))
(overlays-in (line-beginning-position) (line-end-position))))
(when-let (fun (overlay-get ov 'isearch-open-invisible))
(when (invisible-p (overlay-get ov 'invisible))
(funcall fun ov))))
(if (featurep 'org-fold) (org-fold-show-set-visibility 'local)))
(defun consult--invisible-open-temporarily ()
"Temporarily open overlays which hide the current line.
See `isearch-open-necessary-overlays' and `isearch-open-overlay-temporary'."
(let (restore)
(dolist (ov (let ((inhibit-field-text-motion t))
(overlays-in (line-beginning-position) (line-end-position))))
(let ((inv (overlay-get ov 'invisible)))
(when (and (invisible-p inv) (overlay-get ov 'isearch-open-invisible))
(push (if-let (fun (overlay-get ov 'isearch-open-invisible-temporary))
(progn
(funcall fun ov nil)
(lambda () (funcall fun ov t)))
(overlay-put ov 'invisible nil)
(lambda () (overlay-put ov 'invisible inv)))
restore))))
restore)
(if (featurep 'org-fold)
(save-excursion
(org-fold-save-outline-visibility nil
(org-fold-show-set-visibility 'local)))))UPDATE: If this is ok, I'm going to create GitHub PR now. If need to modify, let me know. |
… consult-org-heading
|
I'm not sure why this one is closed (TBH, I didn't carefully read through every comment here). On my machine (with Org mode 9.6) I'm facing this problem - with the default value of But with that, I think, ideally, both - consult and org-appear should work with either type of I did the following for consult (keeping It can be a bit simpler in Doom: |
|
@stardiviner did you end up having any ideas about restoring the expanded state? It would be quite nice if this worked 🙂. |
|
No, waiting for org-mode maintainer and consult maintainer add this API. |
Could you please elaborate what kind of API you are looking fore. Ideally, please sent an email to Org ML. |
|
@yantar92 Currently we have two simple functions Lines 1239 to 1263 in aaba2b0 Multiple packages are affected by this, at least evil, ctrlf, swiper and helm-swoop/occur. Maybe these guys figured out the solution? For now the fix is to simply continue to use overlay-based folding. (setq org-fold-core-style 'overlays) |
|
@yantar92 I noticed that you already provide |
|
A tentative API is below. Let me know if it is sufficient. (cl-defun org-fold-core-get-regions (&key specs from to with-markers relative)
"Find all the folded regions in current buffer.
Each element of the returned list represent folded region boundaries
and the folding spec: (BEG END SPEC).
Search folds intersecting with (FROM TO) buffer region if FROM and TO
are provided.
If FROM is non-nil and TO is nil, search the folded regions at FROM.
When SPECS is non-nil it should be a list of folding specs or a symbol.
Only return the matching fold types.
When WITH-MARKERS is non-nil, use markers to represent region
boundaries.
When RELATIVE is a buffer position, regions boundaries are given
relative to that position.
When RELATIVE is t, use FROM as the position.
WITH-MARKERS must be nil when RELATIVE is non-nil."
(when (and relative with-markers)
(error "Cannot use markers in non-absolute region boundaries"))
(when (eq relative t)
(setq relative from))
(unless (listp specs) (setq specs (list specs)))
(let (regions region mk-region)
(org-with-wide-buffer
(when (and from (not to)) (setq to (point-max)))
(unless from (setq from (point-min)))
(dolist (spec (or specs (org-fold-core-folding-spec-list)) regions)
(goto-char from)
(catch :exit
(while (or (not to) (< (point) to))
(when (org-fold-core-get-folding-spec spec)
(setq region (org-fold-core-get-region-at-point spec))
(when relative
(cl-decf (car region) relative)
(cl-decf (cdr region) relative))
(if (not with-markers)
(setq mk-region `(,(car region) ,(cdr region) ,spec))
(setq mk-region `(,(make-marker) ,(make-marker) ,spec))
(move-marker (nth 0 mk-region) (car region))
(move-marker (nth 1 mk-region) (cdr region)))
(push mk-region regions))
(unless to (throw :exit nil))
(goto-char (org-fold-core-next-folding-state-change spec nil to))))))))
(cl-defmacro org-fold-core-regions (regions &key override clean-markers relative)
"Fold every region in REGIONS list in current buffer.
Each region in the list is a list (BEG END SPEC-OR-ALIAS) describing
region and folding spec to be applied.
When optional argument OVERRIDE is non-nil, clear folding state in the
buffer first.
When optional argument CLEAN-MARKERS is non-nil, clear markers used to
mark region boundaries in REGIONS.
When optional argument RELATIVE is non-nil, it must be a buffer
position. REGION boundaries are then treated as relative distances
from that position."
`(org-with-wide-buffer
(when ,override (org-fold-core-region (point-min) (point-max) nil))
(pcase-dolist (`(,beg ,end ,spec) (delq nil ,regions))
(if ,relative
(org-fold-core-region (+ ,relative beg) (+ ,relative end) t spec)
(org-fold-core-region beg end t spec))
(when ,clean-markers
(when (markerp beg) (set-marker beg nil))
(when (markerp end) (set-marker end nil))))))
(defmacro org-fold-core-save-visibility (use-markers &rest body)
"Save and restore folding state around BODY.
If USE-MARKERS is non-nil, use markers for the positions. This
means that the buffer may change while running BODY, but it also
means that the buffer should stay alive during the operation,
because otherwise all these markers will point to nowhere."
(declare (debug (form body)) (indent 1))
(org-with-gensyms (regions)
`(let* ((,regions ,(org-fold-core-get-regions :with-markers use-markers)))
(unwind-protect (progn ,@body)
(org-fold-core-regions ,regions :override t :clean-markers t))))) |
|
@yantar92 Thank you very much! Unfortunately I didn't have time yet to experiment with the API for Consult, but maybe someone else could look into this (maybe @tecosaur)? I wonder if you plan to also expose some higher level API such that I don't have to know the folding mechanism, which is an implementation detail (overlay vs properties)? |
It is already implementation-agnostic. The only undefined behaviour is what happens when you copy the text. Overlays will not be carried around while text properties will be. |
This involved hunting down a bug in newer org versions (see minad/consult#563), wrapping the call in my own function to cycle the heading open, and adding it to my scroll-to-top advice list.
minad
changed the title
consult-org-heading does not auto reveal on new added org-fold feature of org-mode
|
Just upgraded to Emacs 29. The emacs-29 branch doesn't include Org 9.6 yet. Hopefully it will when I compile Emacs the next time. |
|
@yantar92 I implemented unfolding/refolding support using the new APIs provided by you. Thank you very much for that. The API works well, but I have a few remarks:
Lines 1369 to 1378 in 68c1c07 |
|
After having played around more with the provided Org API, I become more critical of what we've got. I think Lines 1370 to 1374 in 633b342 |
|
Daniel Mendler ***@***.***> writes:
@yantar92 I implemented unfolding/refolding support using the new APIs provided by you. Thank you very much for that. The API works well, but I have a few remarks:
- There seems to be a superfluous comma in `org-fold-core-save-visibility`, see https://github.com/emacs-mirror/emacs/blob/dc0f2ec2dbf101a7b755db106ff0e53cbca3b23c/lisp/org/org-fold-core.el#L1061.
There is no issue there. See `org-with-gensyms'.
- It would be better if `org-fold-core-regions` was a function. First this would lead to more compact byte code at the call sites. Furthermore it would allow external packages to use the API without loading org-fold-core at compile time. In Consult I cannot use this macro currently. Instead I have to iterate myself over the regions. Generally I would avoid macros where a function would do, for better debugging and since it facilitates such dynamic API checks at runtime.
Feel free to send a patch to Org ML.
:from (point-min) :to (point-max))
is redundant
…--
Ihor Radchenko // yantar92,
Org mode contributor,
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>
|
|
Daniel Mendler ***@***.***> writes:
After having played around more with the provided Org API, I become more critical of what we've got. I think `org-fold-show-set-visibility` could be better. What we actually want is an API which unfolds the regions at point and also returns a list of regions such that they can be restored. This will be more efficient than the current approach, where ALL the regions are saved and restored. See the comment here:
https://github.com/minad/consult/blob/633b34244faabd299fe5ec5b213fba0a5f631cd6/consult.el#L1370-L1374
As discussed in IRC room, tracking which regions are unfolded by BODY is
not reliable.
…--
Ihor Radchenko // yantar92,
Org mode contributor,
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>
|
|
On 12/8/22 12:29, yantar92 wrote:
Daniel Mendler ***@***.***> writes:
> After having played around more with the provided Org API, I become
more critical of what we've got. I think `org-fold-show-set-visibility`
could be better. What we actually want is an API which unfolds the
regions at point and also returns a list of regions such that they can
be restored. This will be more efficient than the current approach,
where ALL the regions are saved and restored. See the comment here:
>
>
https://github.com/minad/consult/blob/633b34244faabd299fe5ec5b213fba0a5f631cd6/consult.el#L1370-L1374
As discussed in IRC room, tracking which regions are unfolded by BODY is
not reliable.
I understand your point, but I do not agree with your statement in
generality. For my use case it would be reliable, since all folding
would go through the transactional API. It is only unreliable if
uncontrolled folding or unfolding is performed within the guarded block
(not going through the transactional API).
|
|
Daniel Mendler ***@***.***> writes:
> As discussed in IRC room, tracking which regions are unfolded by BODY is
> not reliable.
I understand your point, but I do not agree with your statement in
generality. For my use case it would be reliable, since all folding
would go through the transactional API. It is only unreliable if
uncontrolled folding or unfolding is performed within the guarded block
(not going through the transactional API).
If you see a measurable performance impact, feel free to propose an
alternative version of the API function. We can have both, why not.
…--
Ihor Radchenko // yantar92,
Org mode contributor,
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>
|
|
On 12/8/22 12:28, yantar92 wrote:
> - There seems to be a superfluous comma in
`org-fold-core-save-visibility`, see
https://github.com/emacs-mirror/emacs/blob/dc0f2ec2dbf101a7b755db106ff0e53cbca3b23c/lisp/org/org-fold-core.el#L1061.
There is no issue there. See `org-with-gensyms'.
Thanks for the clarification.
> - It would be better if `org-fold-core-regions` was a function. First
this would lead to more compact byte code at the call sites. Furthermore
it would allow external packages to use the API without loading
org-fold-core at compile time. In Consult I cannot use this macro
currently. Instead I have to iterate myself over the regions. Generally
I would avoid macros where a function would do, for better debugging and
since it facilitates such dynamic API checks at runtime.
Feel free to send a patch to Org ML.
Okay, I will do that.
https://github.com/minad/consult/blob/68c1c07a9bdea0b806b2762c5b17296de7b2eeb8/consult.el#L1369-L1378
:from (point-min) :to (point-max))
is redundant
No, it is not. I tried omitting these arguments and the
folding/unfolding stops working in Consult. :(
|
|
On 12/8/22 12:54, yantar92 wrote:
Daniel Mendler ***@***.***> writes:
>> As discussed in IRC room, tracking which regions are unfolded by BODY is
>> not reliable.
>
> I understand your point, but I do not agree with your statement in
> generality. For my use case it would be reliable, since all folding
> would go through the transactional API. It is only unreliable if
> uncontrolled folding or unfolding is performed within the guarded block
> (not going through the transactional API).
If you see a measurable performance impact, feel free to propose an
alternative version of the API function. We can have both, why not.
Yes, as we discussed, such a transactional API would be nice to have but
is probably not necessary for now. If performance issues are observed in
the future, I will revisit this.
|
|
On 12/8/22 12:58, yantar92 wrote:
Daniel Mendler ***@***.***> writes:
>> :from (point-min) :to (point-max))
>>
>> is redundant
>
> No, it is not. I tried omitting these arguments and the
> folding/unfolding stops working in Consult. :(
Hmm. I think you missed parts of the docstring:
When RELATIVE is a buffer position, region boundaries are given
relative to that position.
When RELATIVE is t, use FROM as the position.
I don't understand. I don't use RELATIVE.
|
|
Daniel Mendler ***@***.***> writes:
> :from (point-min) :to (point-max))
>
> is redundant
No, it is not. I tried omitting these arguments and the
folding/unfolding stops working in Consult. :(
…--
Ihor Radchenko // yantar92,
Org mode contributor,
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>
|
|
I don't understand. I don't use RELATIVE.
Never mind that. I misread the code.
|
|
On 12/8/22 13:07, yantar92 wrote:
Daniel Mendler ***@***.***> writes:
>> :from (point-min) :to (point-max))
>>
>> is redundant
>
> No, it is not. I tried omitting these arguments and the
> folding/unfolding stops working in Consult. :(
Fixed on bugfix.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=8b6df19dc
Okay, great! I assume the fix will also be part of Emacs 29 and the next
Org 9.6.1 bugfix release, such that I can drop the arguments soon?
Daniel
|
|
Daniel Mendler ***@***.***> writes:
> Fixed on bugfix.
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=8b6df19dc
Okay, great! I assume the fix will also be part of Emacs 29 and the next
Org 9.6.1 bugfix release, such that I can drop the arguments soon?
Yup. I think Org 9.6.1 will be released this weekend. There is a security
fix, so it should be done soon.
…--
Ihor Radchenko // yantar92,
Org mode contributor,
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>
|
With the latest org-mode source code (on commit "782a66192") which added the new "org-fold" feature. The command
consult-org-headingdoes not auto reveal the entry or heading selected in consult candidates.I recorded a gif screencast here to show this behavior.
Screen.Recording.2022-04-29.at.09.10.27-converted.mov
The text was updated successfully, but these errors were encountered: