piuccio / sublime-esformatter

JavaScript formatter plugin for Sublime Text
MIT License
28 stars 20 forks source link

Reading .esformatter from project doesn't (seem) to work #44

Closed jzaefferer closed 9 years ago

jzaefferer commented 9 years ago

I have a project with a .esformatter file like this:

{
    "root": true,
    "preset": "jquery"
}

Apply the plugin to any js file within that project uses the default configuration (or whatever I've set via format_options in EsFormatter.sublime-settings. The console doesn't output anything when I format, so I currently have no idea how to debug this - likely enough that I'm doing something wrong.

webstacker commented 9 years ago

I have the save issue, .esformatter seems to be ignored. Did you manage to get it working?

sircharleswatson commented 9 years ago

same issue here. This definitely needs to be resolved.

piuccio commented 9 years ago

This should now work properly :wink:

webstacker commented 9 years ago

I'm still getting the same issue. I set up a fresh VM with the following configuration but .esformatter is ignored. Am I missing something?

Windows 7 Sublime Text 3 Build 3083 esformatter 0.7.1 (installed locally in project) EsFormatter 1.6.1

EsFormatter.sublime-settings:

{
    // Format the file when saved
    "format_on_save": false,

    // Path to the node executable, by default it tries to resolve the executable
    // for the command 'node' or 'nodejs'. Change this value to specify your own path
    "nodejs_path": null,

    // EsFormatter specific options
    // default are specified here https://github.com/millermedeiros/esformatter/blob/master/lib/preset/default.json
    "format_options" : {}
}

EsFormatter.sublime-settings--User: Empty file

.esformatter (root of project):

{
    "root": true,
    "preset": "jquery"
}
piuccio commented 9 years ago

:cry: I haven't tried on windows, I'll have a look one day

webstacker commented 9 years ago

Is there any way to manually specify what .esformatter to use? I can't see anything in the docs.

Should this issue be reopened until it's resolved?

piuccio commented 9 years ago

The logic now (works on my mac) is to look up .esformatter from the file path. esformatter recurses up the parent folder until it finds one. There must be a .esformatter somewhere up in the tree.

I might be able to add a configuration option to specify the path of .esformatter

webstacker commented 9 years ago

Yeah I'm not sure where it's looking, there's nothing in the console to help debug. Regardless of where I put .esformatter it just doesn't see it. It's a great plugin otherwise, thanks for the ongoing work.

JiaLiu commented 9 years ago

Meet same issue with @webstacker in Windows. I saw the effort from @piuccio but in

var basedir = filePath ? path.dirname(filePath) : process.cwd();

"basedir" is evaluated to "." and its "parentdir" is resolved as "/" though my "filePath" is "C:\xx\xx\xx.js".

No problem if I independently run "esformatter" in CLI.

JiaLiu commented 9 years ago

I think I find the root cause of not working in Windows. "lib/esformatter.js" includes a implementation of "path" module but this "path" loses the part for Windows, only including "posix" part. Please refer https://github.com/jinder/path/blob/master/path.js.

And it seems that "lib/esformatter.js" is generated by "browserify". Maybe "browserify" provides this broken "path" implementation?

natorojr commented 9 years ago

Why is this sublime plugin using its own baked-in version of esformatter? Couldn't you just call the esformatter executable that's installed globally by npm? That's how it seems like other plugins are doing it in the ecosystem. For example, Sublime-contrib-eslint just calls out to the already installed eslint program.

https://github.com/roadhump/SublimeLinter-eslint/blob/master/linter.py#L25

This is a much more flexible implementation and just works as esformatter would normally work when called from the command line. I believe this would solve most (if not all) of the issues being reported regarding loading .esformatter and plugins.

piuccio commented 9 years ago

@natorojr Valid point.

I included esformatter because I didn't like having an external dependency. You can install the plugin and it works out of the box.

Removing the bundled version means it won't be possible to change esformatter configuration from sublime.

The current implementation, anyway, gives priority to the gloabal instance of esformatter. The problem here is that on Windows that mechanism doesn't work...

natorojr commented 9 years ago

My .esformatter config looks like:

{
    "plugins": [
        "esformatter-braces",
        "esformatter-dot-notation",
        "esformatter-eol-last",
        "esformatter-jsx",
        "esformatter-parseint",
        "esformatter-quote-props",
        "esformatter-quotes",
        "esformatter-remove-trailing-commas",
        "esformatter-semicolons",
        "esformatter-spaced-lined-comment",
        "esformatter-use-strict"
    ],
    "indent": {
        "value": "    "
    },
    "jsx": {
        "formatJSX": true,
        "spaceInJSXExpressionContainers": "",
        "htmlOptions": {}
    }
}

When I hit Cmd-Alt-f I get errors: Error (1):Unable to format

I've tried installing the plugins both globally and locally to the project resulting in the same errors.

After further debugging, I see that the code is calling: https://github.com/piuccio/sublime-esformatter/blob/master/EsFormatter.py#L126

How do I get it to give priority to the global instance of esformatter as you suggested above?

Note, everything works fine when I run esformatter from the command line with the same exact .esformatter config file.

piuccio commented 9 years ago

I pushed some changes on branch rcWin that should fix the issue on windows. I can't publish new plugin versions from this machine, so I'll do it next week.

piuccio commented 9 years ago

I've just pushed a new version, 1.6.2, that should fix the issue on windows. I don't guarantee plugins work.