Don't pollute the outline- namespace

[thblt]

This PR removes all definitions on the outline- pseudo-namespace from outshine.el

• Functions and variables already defined by outline.el are completely removed.
• Other functions and variables are renamed from outline-* to outshine-outline-*.

I've tested the changes, and they seem to work perfectly.

The detail of what is renamed and removed is in each commit message.

This PR is a superset of #16. It can completely replace it, but the changes here go beyond a simple bug fix --- they're a complicated bug fix, as they even fix future bugs 🔮 :-)

Thanks!

[thblt]

OK, done, sorry for the noise. I have updated the original description,

[thblt]

OK, done, sorry for the noise. I have updated the original description,

[alphapapa]

Thank you very much. I don't have time to review this right now, but it looks good, and it needs to be done.

What about renaming the symbols by simply changing outline to outshine instead of prepending outshine? outshine-* seems sufficient, rather than outshine-outline-*. What do you think?

[alphapapa]

Thank you very much. I don't have time to review this right now, but it looks good, and it needs to be done.

What about renaming the symbols by simply changing outline to outshine instead of prepending outshine? outshine-* seems sufficient, rather than outshine-outline-*. What do you think?

[thblt]

What about renaming the symbols by simply changing outline to outshine

I don't really have an opinion on that matter, tbh. The rationale behind the double prefix was that these are functions in outshine but which apply on outline-[minor-]mode, regardless of whether outshine is activated. (Also, it was easier to add than to change a prefix)

For future reference, here's the code I used, so we can adjust it to repeat the changes with any rename strategy you'll prefer:

(mapcar
 (lambda (f)
   (goto-char (point-min))
   (while (re-search-forward (format "\\<%s\\>" f) nil t)
     (replace-match (format "outshine-%s" f))))
 '(outline-body-p
   outline-body-visible-p
   outline-change-heading
   outline-change-level
   outline-cleanup-match
   outline-cycle
   outline-cycle-emulate-tab
   outline-headings-atom
   outline-headings-list
   outline-hide-more
   outline-next-line
   outline-show-more
   outline-static-level-p
   outline-subheadings-p
   outline-subheadings-visible-p))

(there' a bug I just noticed, because outline-cycle-emulate-tab matches \<outline-cycle\> --- - is a word boundary. I --force pushed a fixed commit)

In a few minutes (as soon as I find back my PGP smartcard :) I'm going to push two a new commit on this branch to remove outline-mode-easy-bindings.el entirely --- the file is marked as obsolete and does the same nasty outline- overrides we're removing in this PR; and I'll rebase my branch onto master.

[thblt]

What about renaming the symbols by simply changing outline to outshine

I don't really have an opinion on that matter, tbh. The rationale behind the double prefix was that these are functions in outshine but which apply on outline-[minor-]mode, regardless of whether outshine is activated. (Also, it was easier to add than to change a prefix)

For future reference, here's the code I used, so we can adjust it to repeat the changes with any rename strategy you'll prefer:

(mapcar
 (lambda (f)
   (goto-char (point-min))
   (while (re-search-forward (format "\\<%s\\>" f) nil t)
     (replace-match (format "outshine-%s" f))))
 '(outline-body-p
   outline-body-visible-p
   outline-change-heading
   outline-change-level
   outline-cleanup-match
   outline-cycle
   outline-cycle-emulate-tab
   outline-headings-atom
   outline-headings-list
   outline-hide-more
   outline-next-line
   outline-show-more
   outline-static-level-p
   outline-subheadings-p
   outline-subheadings-visible-p))

(there' a bug I just noticed, because outline-cycle-emulate-tab matches \<outline-cycle\> --- - is a word boundary. I --force pushed a fixed commit)

In a few minutes (as soon as I find back my PGP smartcard :) I'm going to push two a new commit on this branch to remove outline-mode-easy-bindings.el entirely --- the file is marked as obsolete and does the same nasty outline- overrides we're removing in this PR; and I'll rebase my branch onto master.

[thblt]

This is mergeable again.

[thblt]

This is mergeable again.

[alphapapa]

Thank you very much!

I'm tempted to go ahead and merge this without further question, but I did think of one: It seems like merging this could result in a change in behavior for existing users, because although this package shouldn't override these functions, it has been. So users who upgrade to this version will suddenly have the "real" outline functions again, which is ultimately what we want, but they may have subtle differences in behavior.

You said you tested this PR (which I greatly appreciate!), but I'm curious: does that mean that you tested these commands and functions, which are effectively new ones now, or that you tested the outline- functions, which should no longer be overridden?

I'm not sure if all this matters, because this is ultimately the right thing to do. But I do wonder if we need to do anything to smooth over any kind of transition. What do you think?

Thanks.

[alphapapa]

Thank you very much!

I'm tempted to go ahead and merge this without further question, but I did think of one: It seems like merging this could result in a change in behavior for existing users, because although this package shouldn't override these functions, it has been. So users who upgrade to this version will suddenly have the "real" outline functions again, which is ultimately what we want, but they may have subtle differences in behavior.

You said you tested this PR (which I greatly appreciate!), but I'm curious: does that mean that you tested these commands and functions, which are effectively new ones now, or that you tested the outline- functions, which should no longer be overridden?

I'm not sure if all this matters, because this is ultimately the right thing to do. But I do wonder if we need to do anything to smooth over any kind of transition. What do you think?

Thanks.

[thblt]

they may have subtle differences in behavior.

I'm not too worried about that, i suspect most of the changes will be bugfixes. The worse that may (and will) happen is keybindings to renamed functions that will suddenly become invalid. But it can't really be avoided.

does that mean that you tested these commands and functions, which are effectively new ones now, or that you tested the outline- functions, which should no longer be overridden?

I tested a reasonable set of both outshine-outline-* and outline- (after outshine-hook-function), and everything seems to work as expected. More generally, I've been using my branch instead of your HEAD (i manage my packages with Borg, so branch switch is a transparent operations from Emacs' PoV) since the beginning of this PR, and except for the original, overdestructive attempt, I haven't noticed any issue. But outshine is complex, and although I'm reasonably confident nothing catastrophic should arise from these changes, I cannot guarantee there won't be some quirks along the way :)

But I do wonder if we need to do anything to smooth over any kind of transition

IMHO, the correct thing to do is advertise the merge as soon as it's done (and before Melpa build completes), so people can be warned their next update may require some manual changes in their config. I can draft a Reddit post, for example, the Reddit Emacs community is quite active (much more than the emacs-tangents mailing list anyway)

(Also, maybe just don't tag a new release before one or two weeks after the merge, so people who use melpa-stable won't receive an undertested version)

[thblt]

they may have subtle differences in behavior.

I'm not too worried about that, i suspect most of the changes will be bugfixes. The worse that may (and will) happen is keybindings to renamed functions that will suddenly become invalid. But it can't really be avoided.

does that mean that you tested these commands and functions, which are effectively new ones now, or that you tested the outline- functions, which should no longer be overridden?

I tested a reasonable set of both outshine-outline-* and outline- (after outshine-hook-function), and everything seems to work as expected. More generally, I've been using my branch instead of your HEAD (i manage my packages with Borg, so branch switch is a transparent operations from Emacs' PoV) since the beginning of this PR, and except for the original, overdestructive attempt, I haven't noticed any issue. But outshine is complex, and although I'm reasonably confident nothing catastrophic should arise from these changes, I cannot guarantee there won't be some quirks along the way :)

But I do wonder if we need to do anything to smooth over any kind of transition

IMHO, the correct thing to do is advertise the merge as soon as it's done (and before Melpa build completes), so people can be warned their next update may require some manual changes in their config. I can draft a Reddit post, for example, the Reddit Emacs community is quite active (much more than the emacs-tangents mailing list anyway)

(Also, maybe just don't tag a new release before one or two weeks after the merge, so people who use melpa-stable won't receive an undertested version)

[alphapapa]

Hi,

I'm sorry I neglected this for a while.

IMHO, the correct thing to do is advertise the merge as soon as it's done (and before Melpa build completes), so people can be warned their next update may require some manual changes in their config. I can draft a Reddit post, for example, the Reddit Emacs community is quite active (much more than the emacs-tangents mailing list anyway)

Would you be willing to have a "flag day" with a few weeks advance notice? e.g. we could post to Reddit (and maybe the Org mailing list), with a date decided in advance, explaining the changes that will be made and when, and a link to the branch that will be merged. Then on that day, we merge that branch to master and let MELPA build the new package.

That might be overkill, but this package has been around for a long time, and a lot of users have installed it, and I would like to avoid breaking anyone's config with weird errors without notice. That happens too much on MELPA already. Few users keep their packages in version control, which would allow them to rollback to the previous version and fix the breakage at a later time, so most people who would experience such breakage (if there is any) might be stuck with fixing it at the time.

Thanks for your work on this!

[alphapapa]

Hi,

I'm sorry I neglected this for a while.

IMHO, the correct thing to do is advertise the merge as soon as it's done (and before Melpa build completes), so people can be warned their next update may require some manual changes in their config. I can draft a Reddit post, for example, the Reddit Emacs community is quite active (much more than the emacs-tangents mailing list anyway)

Would you be willing to have a "flag day" with a few weeks advance notice? e.g. we could post to Reddit (and maybe the Org mailing list), with a date decided in advance, explaining the changes that will be made and when, and a link to the branch that will be merged. Then on that day, we merge that branch to master and let MELPA build the new package.

That might be overkill, but this package has been around for a long time, and a lot of users have installed it, and I would like to avoid breaking anyone's config with weird errors without notice. That happens too much on MELPA already. Few users keep their packages in version control, which would allow them to rollback to the previous version and fix the breakage at a later time, so most people who would experience such breakage (if there is any) might be stuck with fixing it at the time.

Thanks for your work on this!

[thblt]

@alphapapa that's a good idea, I can draft an announcement, hopefully this weekend, and we could merge in something like six weeks? I'm a bit too busy right now, so publishing the annonce somewhere in the next two weeks for a merge in mid-october would be ideal to allow me to follow up on eventual issues, if that's OK with you too?

[thblt]

@alphapapa that's a good idea, I can draft an announcement, hopefully this weekend, and we could merge in something like six weeks? I'm a bit too busy right now, so publishing the annonce somewhere in the next two weeks for a merge in mid-october would be ideal to allow me to follow up on eventual issues, if that's OK with you too?

[alphapapa]

Sure, that sounds fine. Thank you for your work on this.

[alphapapa]

Sure, that sounds fine. Thank you for your work on this.

[thblt]

Later than expected, here's an announcment draft. Tell me what you think, and don't hesitate to correct my English ;)

Outshine users: big changes ahead, some testing needed!

The first of two major PRs is about to be merged on the Outshine package. For historical reasons, Outshine used to (re)define a lot of functions and variables in the outline- pseudo namespace, conflicting with the built-in Outline it extends, and reintroducing long-fixed bugs in the process (because some of the outline-* functions it defined were actually provided by Outline, but Outshine redefined them using older definitions!). This PR removes most of these definitions, and renames those that no other package provides. This is the first step in making Outshine a polite citizen of modern Emacs world.

We (/u/github-alphapapa and myself) believe the changes should have absolutely no effect beyond fixing some bugs and quirks in Outshine but... it's a big refactoring, so you never know. We thus kindly ask Outshine users to give thblt's leave-outshine-alone branch a try before it gets merged, and report any issue you encounter --- try to make it reproducible if you can, but report anyway!

Merge will happen on October 20th, 2018, around 12:00 UTC, and should be picked up by Melpa the usual way, and will be tagged so that Melpa Stable picks it up as well.

Thanks!
Adam (Alphapapa) & Thibault (Thblt), co-maintainers of Outshine

---

PS: If you're feeling especially adventurous, you may even give a try to the second big PR. This one will turn Outshine into a real minor mode, isolating its effects using the regular Emacs mechanisms, instead of the big override it currently is. It may be a bit more tricky, but it will be merged later, not before November. If you're interested, begin reading here.

[thblt]

Later than expected, here's an announcment draft. Tell me what you think, and don't hesitate to correct my English ;)

Outshine users: big changes ahead, some testing needed!

The first of two major PRs is about to be merged on the Outshine package. For historical reasons, Outshine used to (re)define a lot of functions and variables in the outline- pseudo namespace, conflicting with the built-in Outline it extends, and reintroducing long-fixed bugs in the process (because some of the outline-* functions it defined were actually provided by Outline, but Outshine redefined them using older definitions!). This PR removes most of these definitions, and renames those that no other package provides. This is the first step in making Outshine a polite citizen of modern Emacs world.

We (/u/github-alphapapa and myself) believe the changes should have absolutely no effect beyond fixing some bugs and quirks in Outshine but... it's a big refactoring, so you never know. We thus kindly ask Outshine users to give thblt's leave-outshine-alone branch a try before it gets merged, and report any issue you encounter --- try to make it reproducible if you can, but report anyway!

Merge will happen on October 20th, 2018, around 12:00 UTC, and should be picked up by Melpa the usual way, and will be tagged so that Melpa Stable picks it up as well.

Thanks!
Adam (Alphapapa) & Thibault (Thblt), co-maintainers of Outshine

---

PS: If you're feeling especially adventurous, you may even give a try to the second big PR. This one will turn Outshine into a real minor mode, isolating its effects using the regular Emacs mechanisms, instead of the big override it currently is. It may be a bit more tricky, but it will be merged later, not before November. If you're interested, begin reading here.

[alphapapa]

Hey, that looks great! Thanks very much for writing that up.

[alphapapa]

Hey, that looks great! Thanks very much for writing that up.

[thblt]

The announcement is live!

[thblt]

The announcement is live!

[kaushalmodi]

A cosmetic change suggestion.

• Would you like to refactor the non-interactive and "users should not meddle with these" functions from outshine-foo to outshine--foo?

[kaushalmodi]

A cosmetic change suggestion.

• Would you like to refactor the non-interactive and "users should not meddle with these" functions from outshine-foo to outshine--foo?

[alphapapa]

The announcement is live!

Wonderful, thank you!

A cosmetic change suggestion.

I have no objection.

[alphapapa]

The announcement is live!

Wonderful, thank you!

A cosmetic change suggestion.

I have no objection.

[yuhan0]

This update breaks compatibility with navi-mode, which contains references to outline-promotion-headings and outline-cycle.
These were refactored to outshine-outline-promotion headings and outshine-outline-cycle respectively, and doing a simple search-and-replace inside navi-mode.el seems to have fixed it. I'm willing to submit a PR there but there could be other renamed symbols that I missed?

On another note, I agree with the above comments about how the newly renamed outshine-outline-* functions could be better renamed as outshine-*for better consistency - currently it seems rather arbitrary - e.g. with outshine-outline-cycle vs outshine-cycle-buffer, outshine-next-link vs outshine-outline-next-line ( vs original outline-next-heading).

[yuhan0]

This update breaks compatibility with navi-mode, which contains references to outline-promotion-headings and outline-cycle.
These were refactored to outshine-outline-promotion headings and outshine-outline-cycle respectively, and doing a simple search-and-replace inside navi-mode.el seems to have fixed it. I'm willing to submit a PR there but there could be other renamed symbols that I missed?

On another note, I agree with the above comments about how the newly renamed outshine-outline-* functions could be better renamed as outshine-*for better consistency - currently it seems rather arbitrary - e.g. with outshine-outline-cycle vs outshine-cycle-buffer, outshine-next-link vs outshine-outline-next-line ( vs original outline-next-heading).

[thblt]

@yuhan0

On another note, I agree with the above comments about how the newly renamed outshine-outline-* functions could be better renamed as outshine-*for better consistency

I opened issue #21 to track that.

[thblt]

@yuhan0

On another note, I agree with the above comments about how the newly renamed outshine-outline-* functions could be better renamed as outshine-*for better consistency

I opened issue #21 to track that.

[yuhan0]

Ah yes, sorry about removing the previous comment, I didn't read the thread carefully and realised afterwards that the issue had been raised 😅
In any case I also did a global search and replace on the above outshine-outline functions and it seems to be working without issues so far. Will report back in a few days on the other thread, thanks!

[yuhan0]

Ah yes, sorry about removing the previous comment, I didn't read the thread carefully and realised afterwards that the issue had been raised 😅
In any case I also did a global search and replace on the above outshine-outline functions and it seems to be working without issues so far. Will report back in a few days on the other thread, thanks!

[thblt]

@yuhan0 If you've made the changes could you please send a PR against whichever branch you've modified? Thanks!

[thblt]

@yuhan0 If you've made the changes could you please send a PR against whichever branch you've modified? Thanks!

[yuhan0]

Done :)

Another minor compatibility issue I just picked up - the outorg package has a reference to the deprecated outshine-hook-function, causing it to throw several warnings whenever outorg-edit-as-org is called.
I suppose since these packages (outshine/outorg/navi) are so integrated with each other, the fixes should be merged in some sort of synchronized manner on the 20th October date?
I'm fairly new to all this but would be glad to send simple PRs for the other two packages as well :)

[yuhan0]

Done :)

Another minor compatibility issue I just picked up - the outorg package has a reference to the deprecated outshine-hook-function, causing it to throw several warnings whenever outorg-edit-as-org is called.
I suppose since these packages (outshine/outorg/navi) are so integrated with each other, the fixes should be merged in some sort of synchronized manner on the 20th October date?
I'm fairly new to all this but would be glad to send simple PRs for the other two packages as well :)