Remember folding state of TOC for restoring it

[wasamasa]

Closes #4.

[wasamasa]

Hmm, this does fix the folding issue, but not the scrolling one...

[snosov1]

Thx! Seems good enough!

However, I have a few appeals:

• From your comments on how folding and unfolding works in Org, I kind of get the general idea of what you're doing in toc-org-folded-heading-p, but don't understand it fully. So, please, add some comments that are necessary for understanding. Detailed descriptions are not necessary, just a couple of sentences in while (< pos next-headline) body and something like "fold toc if it was folded before the update" above when foldedp.
• Please, add some ert tests for the toc-org-folded-heading-p, similar to how it's done for other functions in this package.
• And while we're at it, please, change some names as well: toc-org-folded-heading-p -> toc-org-heading-folded-p, foldedp -> was-folded

[wasamasa]

This makes it jump less. I could make it jump not at all by adjusting old-point when a different amount of text is inserted than deleted...

[snosov1]

You mean, if you replace save-excursion with (goto-char old-point) it somewhat helps the scrolling issue?

[wasamasa]

Yes. The basic problem of save-excursion is that once you insert or delete text preceding point, the marker it's using for restoring point isn't adjusted correctly. Therefore I'm saving point to a number and now adjust it for insertions and deletions.

There's still a small gotcha which I'll probably just let be: If you're already viewing the TOC, there's still scrolling effects. Probably something deep down in Org...

[snosov1]

Ok, I've tested the scrolling effect and, honestly, I'm not sure that it became better. Previously, only the scrolling was acting funky and only if the TOC is partly visible (not a very common case, as I would think). Now, the scrolling is better, but the point starts jumping around. Even worse - if, say, you don't see the TOC and you remove a heading - it will still jump, which is unpleasantly surprising.

[wasamasa]

Are you sure you've checked out d641b66 or are you still on ee01a73? The latter has the aforementioned jumping issues on TOC changes, the former doesn't.

edit: Apparently this is only the case if TOC was unfolded...

[snosov1]

Yeah, I was on the wrong commit. Anyway, now, that I've checked out the latest version - the scrolling still seem to be messed up - and the strangest part is that it worked fine just an hour ago. I guess I messed up something with my Emacs instance - will have to look more deeply, but I don't have the time at the moment. So, will do this later.

And to make it easier - I propose to split the 2 subjects - folding and scrolling - into separate PRs. So, please, leave only the folding functionality here, add the test and we'll merge this. For scrolling, please, open up a different PR.

[wasamasa]

Sounds sensible. The folding part works much better than the scrolling hack for me, I'll write a test, remove the scrolling code, squash and force push what I've got.

[snosov1]

Sounds great, thanks!

[wasamasa]

Done. To be honest, after fiddling around a good deal more with save-excursion and friends and running into rather mysterious redisplay bugs (I get a flicker for any of my NIHed versions), I've decided to just drop that part entirely, now that the greatest annoyance (unfolding) has gone. No need from me for an extra pull request.

edit: Though, maybe there is a way to use save-excursion and not run into the insertion/deletion behaviour. I'd merely need to use save-excursion around the unproblematic parts...

edit2: I've noticed the weird scrolling behaviour is caused by the double cycling ._.

edit3: This is so very stupid. At some point during debugging I did realize that the scrolling changes reminded me of recenter in their behaviour, so I did search the sources and found out cycling calls org-cycle-hook which contains org-optimize-window-after-visibility-change which recenters. So, let-binding that to nil does achieve exactly the same as my previous scrolling hack, but with less bugs. Shall I still keep this up for a new pull request or include this trivial change into the current one?

[snosov1]

Shall I still keep this up for a new pull request or include this trivial change into the current one?

Yeah, let's still have a separate PR.

Regarding the PR, I notice a somewhat strange behavior of org-cycle. If I run the tests from command line - everything works fine (as travis shows). But if I run them with M-x ert - your test fails.

To reproduce this "by hand", I open a temporary buffer and insert

* Foo
A foo foos
** Bar
A bar bars
** Baz
But a baz buzzes

After this, if I use TAB to call org-cycle while standing at (point-min), it goes like this:

* Foo...

* Foo
  A foo foos
** Bar...
** Baz...

* Foo
A foo foos
** Bar
A bar bars
** Baz
But a baz buzzes

i.e. it works as intended (in my understanding).

But if I call it via M-x org-cycle, it goes from

* Foo...

to

* Foo
  A foo foos
** Bar...
** Baz...

and then to

* Foo...

again, i.e. the "fully unfolded" state is never reached.

Do you happen to know what's going on?

[wasamasa]

I can reproduce this and I don't know what the hell is going on there considering F1 k TAB tells me hitting TAB runs org-cycle.

[snosov1]

Maybe this has something to do with "interactive/non-interactive" calls, or something. Yeah, but seems pretty weird, indeed.

[wasamasa]

If you use M-: (org-cycle), it works...

edit: The whole code of that function is really weird, it does for example at some point just call "\t" interactively, modifies last-command and manages remembering old fold state. Maybe this is why it behaves differently in batch or minibuffer contexts.

[wasamasa]

So, I think I've found out why this happens. org-cycle-internal-local does compare last-command to this-command to figure out whether it should unfold the header completely, so if I have a half-folded one and somehow affect last-command (like, by using my own rendition of M-x or just moving somewhere else in-between), it will close it instead of unfolding.

Snipping out this part from the function and re-evaluating fixes the interactive tests and general behaviour of the folding function. Guess I'll turn that into a post for http://emacshorrors.com/...

edit: Done.

[snosov1]

Ok, nice =)

However, I still don't really get it. So, org-cycle function depends on last-command state (btw, yank and friends also does that, so it might not be a horrific hack, but an elegant design ;) ). And the claim is that "it's ok to use it from lisp code"?

But if so, why ert doesn't work then?

And, in my understanding, org-cycle is a pretty "high-level" function. Maybe there's some more specialized function to expand a heading and it should be used instead?

[wasamasa]

If you instrument org-cycle-local-internal (which is what org-cycle ends up calling) you'll see that the interactively executed ert is the current command and whatever you've done before that will be the last. Or worse, if you're using a replacement for M-x (like I do), its commands will end up there and break the test.

I don't see the least bit of code suggesting that Org is elegantly designed. As long as you use it non-programmatically, everything is fine, but descending any further is adventurous at best. I can replace org-cycle with org-cycle-local-internal of course, but that won't really change anything. What I haven't tried yet was using (call-interactively 'org-cycle)...

[snosov1]

If you instrument org-cycle-local-internal (which is what org-cycle ends up calling) you'll see that the interactively executed ert is the current command

Ok, I see now.

I don't see the least bit of code suggesting that Org is elegantly designed

It was supposed to be a joke =)

But anyways, now that I've looked into it a bit more - I say, why not to use show-entry/hide-entry (see http://www.gnu.org/software/emacs/manual/html_node/emacs/Outline-Visibility.html)? It seems like that's the way to do this from elisp code.

And while we're at it, please, consider my take on toc-org-heading-folded-p implementation. As far as I can tell, it seems to work (at least for toc-org use case).

(defun toc-org-heading-folded-p ()
    "Non-nil when heading at point is (partially) folded."
    (save-excursion
      (org-back-to-heading)
      (org-end-of-line)
      (org-invisible-p2)))

[wasamasa]

I'm not sure whether the outline commands will suffice since Org does do a whole lot more than them. I'm not sure whether the alternative implementation of toc-org-heading-folded-p will work either as it shouldn't detect a partially folded tree, only a completely folded one. But I'll give these a try because it's worth to see whether I can settle for something simpler to avoid the remaining hacks.

[snosov1]

I understand the concerns, but my thinking is that none of them matters for toc-org.

We will apply all the functions to a heading with a very simple body - without any fancy Org stuff. And (at least in foreseeable future) the heading will always have no sub-headings.

So, yeah, please, have a look - personally, I feel rather good about how simple this can turn out to be.

[wasamasa]

As expected this does indeed not detect a semi-folded tree. I've been testing this on my init.org and if I start out with a semi-folded tree (by hitting TAB once with point on the TOC heading), it gets cycled one more time. So there's that.

hide-entry and show-entry don't know anything about this intermediate state either. I'm not convinced.

[snosov1]

Can you, please, post an example that doesn't work and provide a short description?

[wasamasa]

OK, here's a shortened version of what toc-org does to my init.org:

Folded:

* Table of Contents                                                   :TOC: […]

Semi-folded:

* Table of Contents                                                   :TOC:
 - Preface
 - Init […]
 - Stuff to do
 - Epilogue

Unfolded:

* Table of Contents                                                   :TOC:
 - Preface
 - Init
   - User Interface
   - Emacs annoyances
   - Packages bundled with Emacs
   - Packages outside Emacs
   - Keybinds
   - Hooks
   - Load unpublished packages
   - Set up unpublished packages
   - Post-Init
 - Stuff to do
 - Epilogue

I can transition to each of these states by having point in the TOC heading and hitting TAB. M-x hide-entry does always return to the folded state, no matter whether I start from the semi-folded or unfolded one. M-x show-entry does always return to the unfolded state, no matter whether I start from the folded or semi-folded one. So, if I were to have my TOC heading in semi-folded state, saving would unfold it although my hack should have preserved the folding state of it. And while that may be simple and nicer to look at, it's simply wrong because this isn't outline-mode, but Org.

[snosov1]

Ok, so the issue is that the list may be partially folded (I didn't realize it's possible for plain lists, i.e. not sub-headings). And if we want to be pedantic about it, we have to restore the state exactly as it was.

But then, I don't understand how your solution is better than what I propose.

First, I don't see how your solution handles even this particular case. Like, how does it differentiate between folded and partially folded?

And going further, how it will deal with something like

* Table of Contents                                                   :TOC:
- Preface
- Init
  - User Interface
  - Emacs annoyances
- Stuff to do
  - Packages bundled with Emacs
- Epilogue
  - Packages outside Emacs
  - Keybinds
  - Hooks
  - Load unpublished packages
  - Set up unpublished packages
  - Post-Init

folded to

* Table of Contents                                                   :TOC:
- Preface
- Init...
- Stuff to do
  - Packages bundled with Emacs
- Epilogue...

(first and third items are folded, but the second is not)?

BTW, the statement "I can transition to each of these states by having point in the TOC heading and hitting TAB" doesn't hold for me. If I hit TAB while I'm on a heading (not on a an item of a list) - the heading is either folded or unfolded - I don't see the "partially" folded state in between.

[wasamasa]

Simple, it scans over the entire subtree for text that is folded by checking for the invisible text property and tracks the locations where this property has the outline value (which indicates that this is a hidden subtree instead of, say, a prettified link). For a completely folded tree, only one location is returned, for a completely unfolded tree, no location is returned and for anything in-between, there will be more than one location in the returned list.

I'm not sure why calling org-cycle twice restores this state, but it does for me. Maybe there is some fancy caching going on?

See http://orgmode.org/manual/Global-and-local-cycling.html for how folding should behave. If it doesn't do that for you, chances are you have customized something (like, org-cycle-emulate-tab) or you've uncovered a bug.

[snosov1]

For a completely folded tree, only one location is returned, for a completely unfolded tree, no location is returned and for anything in-between, there will be more than one location in the returned list.

Yeah, I kind of got that from the code. Like, you can differentiate with your implementation, but it doesn't seem, like you actually do.

I'm not sure why calling org-cycle twice restores this state, but it does for me. Maybe there is some fancy caching going on?

Ok, so in your case it seems like org-cycle is doing all the job. But I'm not really convinced.

The main concern is that it doesn't work for me as you describe:

When I hit TAB, it goes through

,-> FOLDED -> CHILDREN -> SUBTREE --.
'-----------------------------------'

according to messages printed. But children and subtree look exactly the same - completely unfolded (that said, I can fold the individual list items).

I don't believe the page you provided describes this particular use case, so I'm not sure what behavior is intended. But even if I run Emacs without any configuration (emacs -Q), the behavior doesn't change.

My setup is:

GNU Emacs 24.5.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.4.2) of 2015-07-03 on sergei-MS-7758 Org-mode version 8.2.10 (release_8.2.10 @ /usr/local/share/emacs/24.5/lisp/org/)

[snosov1]

Now, that we've talked about it so much, it also got me thinking - maybe updating the TOC only if it actually changes is a good idea. It seems like it would diminish this issue (and the scrolling one) to an almost negligible amount even without this "save/restore the folding state" feature.

[wasamasa]

Indeed, for starters it should be sufficient to remember the last state and check whether the current one is different from it. It would get more difficult though if you want to only delete and add the changed parts of the TOC to avoid it becoming unfolded (because deleting everything and reinserting will remove the overlay hiding the subtree, whereas selective edits should be fine).

[snosov1]

The proposed functionality (only updating the toc when it actually changes) has been implemented long ago and seems to work fine. So, the issue remains only in a fraction of cases when the toc actually needs to be changed. I'm inclined to say that this is tolerable and make this PR history. Going over all the hurdles once again doesn't seem to be worth it.

Anyway, thx - it was a cool ride (especially, reading it now, 1.5 years later) =)