Skip to content

[core] Phase out the spacemacs|dotspacemacs-backward-compatibility #17062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

sunlin7
Copy link
Contributor

@sunlin7 sunlin7 commented Jul 4, 2025

Phase out the spacemacs|dotspacemacs-backward-compatibility.

The Phase out the spacemacs|dotspacemacs-backward-compatibility was compiled into the hybrid-mode-autoloads.el, so we can NOT delete it directly, change it to warn.

@sunlin7
Copy link
Contributor Author

sunlin7 commented Jul 7, 2025

Fix #16884 partially.

Copy link
Collaborator

@bcc32 bcc32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the direction of this change, but I'm not sure this method of warning about the outdated macro works as you intended.

@@ -26,7 +26,8 @@
(defmacro spacemacs|dotspacemacs-backward-compatibility (variable default)
"Return `if' sexp for backward compatibility with old dotspacemacs
values."
`(if (boundp ',variable) ,variable ',default))
(warn "The `dotspacemacs-backward-compatibility' will be removed after 2025.
Please reinstall the package which relies on this macro."))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think directly replacing the real macro definition will break any interpreted code which relies on the macro, right? Furthermore, this warning won't get triggered by any compiled code, which already expanded the macro.

@sunlin7 sunlin7 force-pushed the rework-compatible branch from 473b40e to 2eb6192 Compare July 20, 2025 14:07
@sunlin7
Copy link
Contributor Author

sunlin7 commented Jul 20, 2025

@bcc32 You're absolutely right! I had pushed the quoted line, please help review again. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants