added outline-cycle-all, bugfix

[guicho271828]

see comments in the commit

[tj64]

Thanks for the patch, I cannot test this right now, except reading the code, so I trust you and apply the commit anyway.

Are you aware of outshine.el? It includes all of outline-magic with bug fixes as well as much new functionality, and is accompagnied by outorg.el and navi-mode.el. But it does not replace outline-magic, since its focussed on offering 'org-mode outside org-mode'.

[guicho271828]

I wasn't aware of outshine.el. As far as I recall, I found this outline-magic at emacswiki and at that time outshine did not come up in my mind. If it is maintained better than outline-cycle, .... maybe it's the time to switch, thanks to your advice, ironically.

[tarsius]

Almost everything about this is wrong. @tj64 could you please revert this and stop merging stuff without review and any testing? It is better to leave thinks unmerged than to later have to back paddle.

I think I know what problem @guicho271828 tried to fix but don't have the time to fix this properly now either. But not fixing it is better than applying this supposed fix.

Even if this actually was the proper fix, this commit should still not have been merged. It uses spaces for indentation when the rest of the file uses tabs. What should have been explained in the doc-strings instead was added as ugly comments. It uses search-forward-regexp when in the rest of the file the re-search-forward name is used. search-forward-regexp is called without non-nil NOERROR, which is a new bug. It "fixes" a bug, and implements a new command in the same commit. The new command isn't well integrated and instead comes with a doc-string containing "this is how to define a key binding" followed by the same thing but commented out and with a different key (commented with ; where ;; should be used).

I could probably find some more problems -- this should not have been merged.

[guicho271828]

so... I made the points raised by @tarsius fixed. Thank you very much for the review on stylistic errors and the new bug.

Previously, C-u outline-cycle hid every contents of the buffer when (1) the new cycle is the overview (2) the beginning of buffer does not contain the headline and (3) the first headline is not the level-1 headline. It is inconvenient when we define some headlines, e.g., \chapter,\part,\section ..., but not use all of them, because some latex files do not contain \chapter and \part. It should work whatever top level the file is using.

This point could be related to #2, but my #3 was a further fix for it.

This problem is reproducible, see the test repository I specifically made for it @ https://github.com/guicho271828/outline-magic-test . Clone the repo and open the file in that repo, specifically head2-cycleall.txt and head2-cycle.txt, assuming the load-path is already set up for outline-magit. (no need to set it up for outline-magit-test.) Note that this testing is just a hack.

[guicho271828]

#4

[tarsius]

I think I was a bit rude, sorry about that.

I haven't reviewed #4 yet, but will probably do so later today.

[tarsius]

I have dug a bit deeper and have come to the conclusion that outline-magic is obsolete and unnecessary, and actually does more damage than good. It should be deprecated and the README should say so. See https://github.com/emacsmirror/p/issues/47 for more information.

So no review from me.

[guicho271828]

I agree, and "maybe it's the time to switch" as mentioned earlier. hmm..

[tj64]

@tarsius
Ok, sorry for the trouble and thanks for the comments, I actually shouldn't apply untested stuff, I always felt quite nervous about it, but I tried to avoid that outshine et.el. look unmaintained.
I have no time at all right now, and not even a working linux/emacs environment with internet connection.

I hope things will normalize over the year, and I can take care of these libraries again.

@guicho271828
I now reverted you patch and did not apply the new one since tarsius is right, this stuff should be reviewed, tested in trunk, and only then be applied to master.
But your work is not lost, once I have more time again, I will come back and review the code.
In the meantime, you could continue improving outline-magic in your own fork/branch and do prepare pull requests, but I think tarsius is right - outline-magic is kind of obsolete, outshine is much smarter and much more up-to-date.

[tarsius]

Enjoy your offline time!

[guicho271828]

@tj64 no problem, I understand your situation. Thank you for spending the time responding to this.