Skip to content

Fix for Issue #48: Changed the black executable from string to list … #59

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: master
Choose a base branch
from

Conversation

sbroberg
Copy link

@sbroberg sbroberg commented Apr 3, 2024

… to allow 'python -m black' to be used instead of just 'black' for the executable name. This is because black as a standalong executable is not something that is typically supported on windows, but the python module invocation always works, and the zombie processes that were using up file handles for Issue 48 was due to the make-process function failing when it couldn't find a black executable. Changing blacken-executable to the list fixes this problem (assuming the black module is installed).

…string to a list to allow 'python -m black' to be used instead of just 'black' for the executable name. This is because black as a standalong executable is not something that is typically supported on windows, but the python module invocation always works, and the zombie processes that were using up file handles for Issue 48 was due to the make-process function failing when it couldn't find a black executable. Changing blacken-executable to the list fixes this problem (assuming the black module is installed).
@@ -48,6 +48,10 @@
"Name of the executable to run."
:type 'string)

(defcustom blacken-executable-list '(blacken-executable)
"Command (with extra arguments) to invoke black"
:type '(repeat string))
Copy link
Member

Choose a reason for hiding this comment

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

We can turn this into an empty list and then concatenate the list with the executable below.

Copy link
Author

@sbroberg sbroberg Apr 5, 2024

Choose a reason for hiding this comment

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

I did it this way because "python -m black" is equivalent to "black" in that both invoke black with no parameters. Otherwise, to configure it as you suggest would require you to set blacken-executable to "python" and blacken-executable-list (or whatever we'd rename it to) to ("-m" "black"), which seems sort of weird to me.

Alternatively, we can leave "black" in blacken-executable alone and have this empty list precede it, like so:

:command `(,@blacken-python-startup-list ,blacken-executable ,@(blacken-call-args))

and have the user-visible config option the string blacken-python-command-for-invocation. If this string is not nil, we would set blacken-python-startup-list to (blacken-python-command-for-invocation "-m").

This would make it more clear to users what the purpose of this setting is for, even if the implementation is more fiddly. It also has the advantage of leaving people's config intact if for some reason they had previously overridden blacken-executable (although my implementation does as well).

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