Skip to content
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

Fix smartparens workaround in auto-completion #4050

Closed
wants to merge 1 commit into from

Conversation

TheBB
Copy link
Collaborator

@TheBB TheBB commented Dec 3, 2015

This will fail if smartparens is excluded.

@@ -263,18 +264,6 @@
eshell-mode-hook)))
:config
(progn
Copy link
Contributor

Choose a reason for hiding this comment

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

[cosmetic] not necessary anymore

Copy link
Owner

Choose a reason for hiding this comment

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

Actually this is not cosmetic, if the argument for :config or any other keyword is multi-line then progn must be used. It makes the code more readable and easier to work with using paredit like commands or navigation. I'll add this to the conventions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah but' it's not multiline anymore? Or do you also want the (progn … for single commands?

Copy link
Owner

Choose a reason for hiding this comment

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

Oh you are right, if it is a one liner then it is indeed unnecessary. In this case I usually but the keyword and the argument on the same line.

@TheBB
Copy link
Collaborator Author

TheBB commented Dec 3, 2015

Updated. :-)

@syl20bnr
Copy link
Owner

syl20bnr commented Dec 4, 2015

Thank you 👍
Cherry-picked into develop branch, you can safely delete your branch.

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

Successfully merging this pull request may close these issues.

3 participants