bcomnes / sublime-standard-format

:sparkles: Runs standard --fix against the javascript in your ST3 window on save or manually.
https://packagecontrol.io/packages/StandardFormat
MIT License
60 stars 21 forks source link

Prefer project settings if they are set #75

Closed ungive closed 3 years ago

ungive commented 3 years ago

Allows you to set project-specific preferences in your .sublime-project file.

{
  "settings": {
    "standard_format": {
      "format_on_save": true,
      "commands": [["eslint_d", "--stdin", "--fix-to-stdout"]]
    }
  }
}

As a matter of fact, using this plugin with eslint_d (as configured above) formats the file pretty much instantly.

bcomnes commented 3 years ago

I like the command idea especially, but format_on_save should probably stay a user setting no matter what (sometimes, you need to disable for whatever reason). But maybe for sake of simplicity we just treat it the same.

Also eslint_d sounds nice. AFAICT, eslint itself didn't have a stdin format option, and I've been using some other janky ST plugin that actually writes over the file instead of the ST buffer like this one does.

I'll try this out too this week and get it released in some form.

bcomnes commented 3 years ago

Ok so this looks good (code wise) and is working, and fails relatively gracefully when there are obvious issues ('bad package.json, etc), but I'm slightly reluctant to add a new field to package.json on behalf of a sublime text plugin. Thank you for the work and energy! I appreciate it.

I guess, what I would offer as a solution as an alternative to consider before possibly accepting this is this:

Would adding more commands to the commands search array to the default config solve the problem that this also solves?

E.g.:

https://github.com/bcomnes/sublime-standard-format/blob/5b697c92b6e58cf67d4c90bfe96e68f9b4e35d7a/StandardFormat.sublime-settings#L31-L33

changed to something like

{
  "commands": [
        ["standard", "--stdin", "--fix"],
        ["semistandard", "--stdin", "--fix"],
        ["eslint_d", "--stdin", "--fix-to-stdout"]
    ]
}

(plus other common ones).

Users can edit precedence in their sublime user settings, add more and re-arrange precedence in case of multiple tools existing in the same project according to user preference).

I guess the main problem it seems we are solving is two fold:

  1. that sublime standard format more probably works by default, and allow for various other tools that take as similar stdin api as standard.
  2. projects can specify project specific settings that direct sublime text plugins

I think adding more defaults would cover your use case on point 1, and is already sort of solved via user settings of the commands array.

Im reluctant to solve 2 for the following reasons:

I would consider a scenario where you have to have multiple tools installed in the same project that can't be reconciled easy with user config, and would require a project override, but even then, I think I would prefer the sublime project solution if that works (never needed it thus far).

Thoughts?

bcomnes commented 3 years ago

Side note, installing eslint_d to my project, and adding it as a Sublime StandardFormat user command, I get excellent formatting in my eslint work projects now. Very cool. I can retire the ESLint-Formater plugin now. Thanks for that insight!

When the plugin is configured to use other tools, their presence locally in a project stands in as a local config that signals to 'use this tool if found'. Another way to look at it.

ungive commented 3 years ago

Users can edit precedence in their sublime user settings, add more and re-arrange precedence in case of multiple tools existing in the same project according to user preference).

I like that solution, then it remains user-configured and it's not part of the project. But in case the project installed a new binary, then that would be preferred due to the precedence in the ordering. The question would be how much overhead you'd add by looking if each command that comes before the first one working is in the PATH. Depending on how much time it takes, you might want to cache the result for each consecutive call where the PATH didn't change.

Adding fields to package.json just creates more config entropy that we end up living with for years.

If it's already there, then being able to overwrite a user's settings for editor formatting would be preferable I think. This way, when switching to another project you'd either use the configured formatter or your standard one.

With your proposed solution of multiple commands though, you wouldn't need to edit anything.

crosses python json (snake case) with JS/node style json (camelCase), and resolving that is more effort than its worth.

Why the need to resolve it? It's in separate files, and it would be easily resolved by allowing camelCase and snake_case for settings. E.g. implement and then call get_setting('foo_bar_baz') which checks for foo_bar_baz and fooBarBaz.

editor config is very editor specific, and would be better off in sublime project config (I'm not sure that even works, maybe worth a shot?). Think vs-code global->user->project config overrides. I think you can do a similar thing with sublime project files.

Arent' we currently doing this? Configuring in the sublime project config i.e. .sublime-project? I might be misunderstanding things here.

bcomnes commented 3 years ago

Oh wow, I read package.json and not .sublime-project file.

sorry about that, let me re-think this, and if thats the case I will take it.

bcomnes commented 3 years ago

Yeah this looks good to me! Sorry about the confusions.

bcomnes commented 3 years ago

These were all great contributions. Thank you @vonas Please let me know if you have any other ideas. Working with eslint_d lately has been a great addition to my non-standard projects I do at work.

This is landing in 6.3.0 as soon as package control picks it up.