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

Evaluate any non-string, non-special args in apheleia-formatters #50

Closed

Conversation

joshforged
Copy link

file, input, and output are special cases in apheleia-formatters that
are specially expanded when interpreted by apheleia-format-buffer.
This commit makes it so any other symbols will be evaluated.

file, input, and output are special cases in apheleia-formatters that
are specially expanded when interpreted by apheleia-format-buffer.
This commit makes it so any other symbols will be evaluated.
@lassik
Copy link

lassik commented Oct 6, 2021

I advise against this. This is potentially unsafe if the values come from .dir-locals.el.

@joshforged
Copy link
Author

Yeah it's definitely generally unsafe -- but what could actually happen (I actually am not sure)? A user defines some nastiness in their own .dir-locals which gets evaluated? In general the dynamic nature of elisp means that any evaluated configuration code can do nasty things in certain circumstances, so I don't see how this is worse than anything else, as long as it's clear that the formatters alist is code.

@raxod502
Copy link
Member

raxod502 commented Oct 6, 2021

This is perfectly safe. The whole notion of safe file-local variables exists for exactly this reason, and Emacs will refuse to automatically apply a file-local setting for apheleia-formatters unless it is approved by the user. Besides, you could already (setf (alist-get prettier apheleia-formatters) '("sudo" "something-malicious")); this doesn't make things any worse.

The implementation looks fine; could you add a changelog entry and update the docstring?

@mohkale
Copy link
Contributor

mohkale commented Oct 17, 2021

Is there anything I can do to help move this along? I'm running into a need for this with shfmt with regards to current indentation (tabs or space-width) and shell type (bash, sh, zsh, etc.)

Since #51 will conflict with this I could merge the functionality onto there. Thoughts?

Edit:

Okay I've implemented it here. Since I've implemented it on top of #51 I'll avoid creating a PR for it until that's merged. My implementation isn't too different from @joshforged except it can smartly handle when the result of an evaluation is a list of strings or nothing. This lets you have conditionals and multi-flag outputs from an evaluation. See the example in the README with shfmt.

@joshforged
Copy link
Author

@mohkale's solution looks more comprehensive and I've been busy lately -- let's close this PR and use #51 instead.

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

Successfully merging this pull request may close these issues.

None yet

4 participants