bitst0rm-pub / Formatter

🧜‍♀️ A Sublime Text plugin to beautify and minify source code: CSS, SCSS, Sass, HTML, XML, SVG,JS,JavaScript, JSON, GraphQL, Markdown, TypeScript, Vue, Lua, YAML, Go, Perl, PHP, Python, Ruby, Rust, Haskell, Dart, Swift, Crystal, Bash, Shell, SQL, CSV, C, C++, C#, Objective-C, D, Java, Pawn, Julia, Proto, LaTeX, D2, Graphviz, Mermaid, PlantUML, etc
Other
100 stars 20 forks source link

Prettier - Wrong local executable path on Windows #41

Closed bhuynhdev closed 10 months ago

bhuynhdev commented 10 months ago

The Formatter plugin seems to be able to detect local executables from the "node_modules/.bin" folder, but it seems to be using the wrong file on Windows.

Specifically, it is using node_modules\.bin\prettier instead of node_modules\.bin\prettier.cmd (reference: https://github.com/VSCodeVim/Vim/issues/2649), thus leading to issue:

▋[Formatter](Thread-4:common.py#L502): [DEBUG] Global interpreter found: C:\Program Files\nodejs\node.EXE
▋[Formatter](Thread-4:common.py#L488): [DEBUG] Local executable found: C:\Users\users\code-project\node_modules\.bin\prettier
▋[Formatter](Thread-4:common.py#L549): [WARNING] Setting key "config_path" is empty or not of type dict: None
▋[Formatter](Thread-4:common.py#L550): [WARNING] Default core config will be used instead if any.
▋[Formatter](Thread-4:formatter_prettier.py#L62): [DEBUG] Current arguments: ['C:\\Program Files\\nodejs\\node.EXE', 'C:\\Users\\users\\code-project\\node_modules\\.bin\\prettier', '--no-config', '--stdin-filepath', 'C:\\Users\\users\\code-project\\package.json']
▋[Formatter](Thread-4:formatter_prettier.py#L73): [ERROR] File not formatted due to an error (errno=1): "C:\Users\users\code-project\node_modules\.bin\prettier:2

basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")

          ^^^^^^^

SyntaxError: missing ) after argument list

    at internalCompileFunction (node:internal/vm:73:18)

    at wrapSafe (node:internal/modules/cjs/loader:1178:20)

    at Module._compile (node:internal/modules/cjs/loader:1220:27)

    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)

    at Module.load (node:internal/modules/cjs/loader:1119:32)

    at Module._load (node:internal/modules/cjs/loader:960:12)

    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)

    at node:internal/main/run_main_module:23:47

Node.js v18.17.1

"
▋[Formatter](Thread-4:main.py#L298): [DEBUG] Formatting failed. 💔😢💔

I have specified a global prettier executable with the executable_path settings, but it seems to be ignored, and Formatter keeps prioritizing using the local executable, which leads to the above error

"prettier": {
            "disable": false,
            "format_on_save": false,
            "new_file_on_format": false,
            "recursive_folder_format": {
                "enable": false,
                "exclude_folders_regex": ["Spotlight-V100", "temp", "cache", "logs", "^_.*?tits\\$"],
                "exclude_files_regex": ["show_tits.sx", ".*?ball.js", "^._.*?"],
                "exclude_extensions": ["DS_Store", "localized", "TemporaryItems", "Trashes", "db", "ini", "git", "svn", "tmp", "bak"],
                "exclude_syntaxes": []
            },
            "executable_path": "%AppData%/npm/node_modules/prettier/bin/prettier.cjs",
            "syntaxes": ["json", "js", "ts", "jsx", "tsx"]
        },
bitst0rm commented 10 months ago

The detection of the executable was reliant on the information provided in this pull request: https://github.com/bitst0rm-pub/Formatter/pull/36 Prettier seems like to love dancing in the past, from bin-prettier.js, to prettier, now prettier.cmd, ...

I do not have windows, You could test this please: In modules > formatter_prettier.py > line 17: from EXECUTABLES = ['prettier', 'bin-prettier.js'] change to EXECUTABLES = ['prettier.cmd', 'prettier', 'bin-prettier.js']

It is correct, local first, then global prioritizing, therefore the error.

bhuynhdev commented 10 months ago

Adding "prettier.cmd" nor "prettier.ps1" did not fix it

Actually seems like it's not that the executable path is wrong; the actual issue seems to be that Formatter is trying to run these executable using Node.exe whereas those executable files in the .bin folder seems to be bash/Powershell/CMD script that is supposed to be run standalone

The referenced issue from VSCodeVim seems to have come to same conclusion and have made the changes to run the prettier path directly without the node interpreter image

bitst0rm commented 10 months ago

ok thanks. We must adjust so for windows:

`EXECUTABLES = ['prettier.cmd', 'prettier', 'bin-prettier.js']`
...
...

    def get_cmd(self):
        executable = common.get_executable(self.view, self.uid, ['prettier.cmd'], runtime_type='node')
        cmd = [executable]
        if not executable:
            cmd = common.get_head_cmd(self.view, self.uid, INTERPRETERS, EXECUTABLES, runtime_type='node')

        if not cmd:
            return None
bhuynhdev commented 10 months ago

Thanks a lot. Using common.IS_WINDOWS is an awwesome idea. I used

# formatter_prettier.py
def get_cmd(self):
    cmd = []
    if common.IS_WINDOWS:
        executable = common.get_executable(self.view, self.uid, EXECUTABLES, runtime_type='node')
        cmd = [executable]
    else:
        cmd = common.get_head_cmd(self.view, self.uid, INTERPRETERS, EXECUTABLES, runtime_type='node')
    if not cmd:
        return None

And also no need for prettier.cmd in the EXECUTABLES list.

Result:

[DEBUG] Current arguments: ['C:\\Users\\users\\code-project\\node_modules\\.bin\\prettier', '--no-config', '--stdin-filepath', 'C:\\Users\\users\\code-project\\src\\HomePage.jsx']
▋[Formatter](MainThread:main.py#L349): [DEBUG] Replacing text ...
▋[Formatter](Thread-1:main.py#L295): [DEBUG] Formatting successful. 🎉😃🍰

It can be seen that after the changes, Formatter now runs the prettier bin file directly without the node.EXE; therefore, it is able to format the file

bitst0rm commented 10 months ago

aha your log: on windows Prettier does not seem to usenode.exe directly. so the final patch would be:

# formatter_prettier.py
EXECUTABLES = ['prettier', 'bin-prettier.js'] # no changes
...
def get_cmd(self):
    if common.IS_WINDOWS:
        executable = common.get_executable(self.view, self.uid, EXECUTABLES, runtime_type='node')
        cmd = [executable]
    else:
        cmd = common.get_head_cmd(self.view, self.uid, INTERPRETERS, EXECUTABLES, runtime_type='node')
    if not cmd:
        return None

i will release this later, thanks.

bhuynhdev commented 10 months ago

Actually my bad. There is this case where there is no local executable found --> Formatter defaults to the global user executable --> The Global prettier executable is a .cjs file --> Now it needs the node interpreter

Sorry for the mistake

New proposed changes:

# formatter_prettier.py
def get_cmd(self):
    if common.IS_WINDOWS:
        executable = common.get_executable(
            self.view, self.uid, EXECUTABLES, runtime_type="node"
        )
        if not executable.endswith("js"):
            # If the executable is a standalone script, then just run it
            cmd = [executable]
        else:  # Else, it is a JS file then it needs an interpreter
            cmd = common.get_head_cmd(
                self.view, self.uid, INTERPRETERS, EXECUTABLES, runtime_type="node"
            )
    else:
        cmd = common.get_head_cmd(
            self.view, self.uid, INTERPRETERS, EXECUTABLES, runtime_type="node"
        )

    if not cmd:
        return None
bitst0rm commented 10 months ago

On unix it does not have .cjs, your patch works well on unix. It looks awesome to catch all cases, could you pls file a push request so i can merged it. Thanks.