jscheid / prettier.el

Prettier code formatting for Emacs.
GNU General Public License v3.0
168 stars 12 forks source link

[bug] Don't assume "node" exists #119

Closed jcs090218 closed 4 months ago

jcs090218 commented 1 year ago

Describe the bug

Backtrace without "node" being installed:

Debugger entered--Lisp error: (file-missing "Searching for program" "No such file or directory" "node")
  make-process(:name "prettier" :buffer #<buffer *prettier (local)*> :command ("node" "--eval" "6>Number(process.version.split(/[v.]/)[1])&&proces..." "9596"))
  start-process("prettier" #<buffer *prettier (local)*> "node" "--eval" "6>Number(process.version.split(/[v.]/)[1])&&proces..." "9596")
  start-file-process("prettier" #<buffer *prettier (local)*> "node" "--eval" "6>Number(process.version.split(/[v.]/)[1])&&proces..." "9596")
  prettier--create-process(local "node")
  prettier--get-process(t)

I think (executable-find "node") already does the job finding node, what's the intention falling back to "node" on L1684? 🤔

https://github.com/jscheid/prettier.el/blob/e419bb7a916c38a4a62632c49233de1523f41218/prettier.el#L1681-L1684

To Reproduce

When "node" is not found on your machine.

Expected behavior

Without the above error.

Additional context

Just kinda annoying.

jscheid commented 1 year ago

The error message is correct: this package currently needs node, you can't use it without node installed. And I don't think it's a good idea to silently ignore absent node, that would confuse people who are actually trying to use it.

jcs090218 commented 1 year ago

I mean we should defense it from making a process, and print the error message.

(if (executable-find "node")
  ( )  ; do task
 )  ; print error, don't even try to create a process.

Or better, add a defcustom variable so user can point to the node executable they want?

+ (defcustom prettier-node-executable nil  ; default is nil
+   ...)

                (setq prettier-nvm-node-command-cache nvm-node-command)
                nvm-node-command))
-           (executable-find "node")
-           "node")
+           prettier-node-executable  ; user defined has higher privileges
+           (executable-find "node"))
jscheid commented 1 year ago

Keep in mind that this package can be used on a remote server, via tramp. I'm open to a PR for this as long as it doesn't break that feature.