outshine icon indicating copy to clipboard operation
outshine copied to clipboard

Qustion: Why do we need outshine specific cycle commands?

Open rgrinberg opened this issue 7 years ago • 10 comments

org-cycle and org-global-cycle seem to work equally well for me in buffers with outline mode (and outshine) enabled. So why do we need these mode specific bindings?

rgrinberg avatar Apr 29 '17 20:04 rgrinberg

@rgrinberg Good question. You may want to ask the current maintainer: see notice here

priyadarshan avatar Apr 30 '17 06:04 priyadarshan

Unfortunately the new repos don't have an issue tracker

On Sat, Apr 29, 2017, 11:28 PM Priyadarshan [email protected] wrote:

@rgrinberg https://github.com/rgrinberg Good question. You may want to ask to current maintainer: see notice here https://github.com/tj64/outshine#outshine

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tj64/outshine/issues/68#issuecomment-298214284, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIe-59DkCwdyk5RRWE9eZ124pN-Lj2Pks5r1CoEgaJpZM4NMYTx .

rgrinberg avatar Apr 30 '17 06:04 rgrinberg

@alphapapa Would it be all right to enable use of issue tracker for nav, outshine and outorg?

priyadarshan avatar Apr 30 '17 06:04 priyadarshan

Uh, oops. I did not realize they were disabled. I'll turn them on right away.

Looking at the outline-cycle function shows that it is not the same as org-cycle, and there is also outline-cycle-emulate-tab. So while org-cycle may work for you, or may work most of the time, I can't say for certain that it would work in all circumstances that outline-cycle does, or that it will do everything the same way. I assume Thorsten implemented it for a reason. ;)

alphapapa avatar Apr 30 '17 07:04 alphapapa

I really don't remember, but it might as well has just historical reasons. Originally, outshine was just an addition to outline, with no dependency to Org-mode. I'm pretty sure it inherited its cycle command from outline-magic by Carsten Dominik, and I seem to remember it needed some fixes and modifications. As more and more feature requests for Org-mode functionality in Outshine popped up, I recognized that its an uphill battle to reimplement lots of Org-mode stuff for Outshine and looked for ways to just use existing Org functionality (e.g. the outshine-use-outorg command), introducing dependencies to org-mode.

AFAIK outline-cycle is an earlier version of org-cycle by the same author (Carsten Dominik), and one might assume that the later org cycle commands are somehow better. But since the (fixed and improved) outline-cycle command worked seamlessly, I did not see a need to switch or to drop it.

I can only repeat what alphapapa says: "I can't say for certain that it would work in all circumstances that outline-cycle does, or that it will do everything the same way". You might want to switch to org-cycle commands in trunk and ask powerusers to use trunk for a while as a test before merging with master. Or you might as well remember the old saying "don't fix it, if it ain't broken", I think that was my strategy for not touching outline-cycle anymore once it worked as expected.

tj64 avatar Apr 30 '17 14:04 tj64

OK Sure.

Let me try using org-cycle and org-global-cycle instead of outshine-cycle and outshine-cycle-buffer. If the results, are positive I say we move towards deprecation. It should be a small positive change for users. They'll have to bind less keys, won't have to duplicate advice, the behavior will stay consistent. Having less code in outshine isn't a bad thing either.

rgrinberg avatar Apr 30 '17 15:04 rgrinberg

If the results, are positive I say we move towards deprecation. It should be a small positive change for users. They'll have to bind less keys, won't have to duplicate advice, the behavior will stay consistent. Having less code in outshine isn't a bad thing either.

I agree in principle: less code and avoiding duplicate functionality is always better. However, if there's one thing I've learned about Emacs development, it's that there are many edge cases that any one user will never hit. I always find bugs in my packages when other people test them, ones I'd probably have never found. So I want to be very careful to not break the code for any existing users. It may be that the other function works perfectly for you, but there may be other users who would experience immediate breakage, and who knows if they would report a bug or just stop using it.

So I propose this:

  1. @rgrinberg continues testing as he suggested.
  2. If that goes for a few weeks with no problems found, I'll add customization to change these functions, defaulting to the existing ones, but allowing users to switch to the org ones. (Frankly, I seriously doubt anyone will go to the trouble to change to the org ones, because they probably won't even notice the options are available, and won't care if they do. But I think it's the right thing to do to be extra careful.)
  3. If no problems are reported for a while, then I'll do some final checks and then switch the defaults to the org functions. I might also add advice to the org functions to give useful errors in outshine mode, telling users how to switch back in case of problems.
  4. If no problems are reported for a while (a significant while), I'll mark these functions as obsolete. Eventually (eventually!) they will be removed.

On the other hand, if we find that they are necessary for some edge cases, then it will probably be necessary to dig in and understand the edge cases, and perhaps refactor them to use the Org functions as much as possible while working around the edge cases. That would be better than essentially duplicating the whole Org functions.

alphapapa avatar May 01 '17 01:05 alphapapa

Keep in mind that outshine has a special macro for keybindings, since its a minor mode (extension) and not a major mode like Org-mode. IIRC those cycle commands are bound to TAB only when point is on a headline, otherwise the keybinding of the major-mode at hand is active (whatever TAB means in that major-mode).

tj64 avatar May 01 '17 10:05 tj64

I almost forgot to report the results of my experiment:

org-cycle and org-global-cycle seem to be adequate substitutes to outshine's versions of these functions. I did not notice any differences over a course of modest but daily use over a month. I suggest a little more testing by the maintainers followed by a move in favor of org's functions.

rgrinberg avatar May 26 '17 04:05 rgrinberg

Thanks for reporting back, Rudi.

While in general it would be good to avoid redundant code, I don't think this is a change that should be made now. outline-cycle and org-cycle are very different, and ensuring that org-cycle works correctly as a replacement would require extensive manual testing across many major modes. Also, outline-cycle has several outshine-specific customizations that would be lost if it were replaced with org-cycle, which would simply be a loss of functionality and a regression. It may be possible to rewrite outline-cycle in a way that makes better use of org-cycle and avoids duplicating functionality, while preserving the customizations, however, "if it ain't broke, don't fix it" is a rule I'm not willing to break unless the potential benefits are substantial. And as far as I know, doing so wouldn't fix any bugs or make any improvements in functionality, so it would essentially be an exercise in code-churn.

In the event of extensive refactoring, this would probably be worth looking into, but short of that, I think we should leave well enough alone. :)

alphapapa avatar May 27 '17 16:05 alphapapa