federomero / pretty-json

Atom plugin. Format JSON documents.
MIT License
94 stars 23 forks source link

Feature Request: Add .editorconfig support [solved?] #36

Closed vinkla closed 7 years ago

vinkla commented 8 years ago

By default this package follows the default npm indentation of 2 spaces. Wouldn't it be cool if the package could read the .editorconfig file if it exists and use those rules instead?

lexicalunit commented 8 years ago

This is totally doable and would be great! Thanks for the suggestion, I'll try to implement it soon.

vinkla commented 8 years ago

Great :+1:

lexicalunit commented 8 years ago

@vinkla Oh also, note that the plugin does already honor your Atom settings. It looks at your tabLength and softTabs settings.

lexicalunit commented 8 years ago

@vinkla Actually... I think if you install https://atom.io/packages/editorconfig that will automatically pick up your editorconfig settings and apply them to Atom. And that in turn will cause those settings to be picked up by pretty-json :)

:tada: package composition and separation of concerns!

Please let me know if this doesn't work out for you by re-opening this ticket with details.

vinkla commented 8 years ago

Still can't get it to work. Here is a video showing the indentation problem I have: https://vink.la/qWbDOBw

I've the .editorconfig package installed with the following settings:

# http://editorconfig.org
root = true

[*]
indent_style = space
indent_size = 2
end_of_line = lf
charset = utf-8
trim_trailing_whitespace = true
insert_final_newline = true

[*.{json,php}]
indent_size = 4

[*.md]
trim_trailing_whitespace = false

The issue is that it uses 2 spaces instead of 4 which I've specified in my .editorconfig file.

lexicalunit commented 8 years ago

Ah! I bet it's the language specific part that this plugin isn't taking into account. I believe I can fix this.

vinkla commented 8 years ago

:+1:

lexicalunit commented 8 years ago

I just pushed out a branch which should help resolve this issue for you @vinkla. I'll merge it into master and publish a new release once tests pass in CI.

What I've done is pick up the language specific settings in Atom for softTabs and tabLength. If the editorconfig package is setting these values correctly in Atom (I don't know for sure if it is or not), then this new release of pretty-json should pick up those settings.

lexicalunit commented 8 years ago

Alright, just published version 1.2.0. Let me know if this doesn't resolve things for you @vinkla! :)

vinkla commented 8 years ago

Thanks for trying to fix this! Though, I'm still having the same issue with the tab length. Is there anything else I can provide?

lexicalunit commented 8 years ago

There is one more thing that would be very helpful. If you could post your Atom config file. You can open it from the main Atom menu system. It will be a cson formatted file.

It's possible that the editorconfig package isn't setting tab length in the Atom config in the way I expect, or at all. I may be able to either fix it in my code, or open a PR in the editorconfig package to fix/add the feature there.

vinkla commented 8 years ago

Let me know if you need anything else!

"*":
  "atom-autocomplete-php": {}
  "atom-ctags":
    GotoSymbolKey: [
      "cmd"
    ]
  "atom-material-ui": {}
  "autocomplete-plus": {}
  core:
    disabledPackages: [
      "symbols-view"
      "php-hyperclick"
    ]
    ignoredNames: [
      ".git"
      ".vagrant"
      ".idea"
      ".hg"
      ".svn"
      ".DS_Store"
      "._*"
      "Thumbs.db"
    ]
    themes: [
      "one-light-ui"
      "octocat-syntax"
    ]
  "custom-title":
    template: "<% if (filePath) { %><%= filePath %><% } else { %> <%= fileName %> <% } %>"
  docblockr:
    align_tags: "no"
    param_description: false
    return_description: false
    spacer_between_sections: true
  editor:
    fontFamily: "Menlo"
    fontSize: 13
    invisibles: {}
    lineHeight: 1.9
  "exception-reporting":
    userId: "hejsan-hejsan-hejsan"
  "file-icons":
    coloured: false
  fuse: {}
  "fuzzy-finder": {}
  "git-diff": {}
  "language-babel": {}
  linter:
    errorPanelHeight: 22
  "linter-eslint":
    disableWhenNoEslintrcFileInPath: true
  "linter-sass-lint":
    noConfigDisable: true
  "markdown-preview":
    useGitHubStyle: true
  minimap:
    absoluteMode: true
  "one-dark-ui":
    layoutMode: "Compact"
  "one-light-ui":
    layoutMode: "Compact"
  "package-generator": {}
  "php-integrator-autocomplete-plus": {}
  "php-integrator-base": {}
  "php-integrator-navigation": {}
  "pretty-json": {}
  react: {}
  "recent-files-fuzzy-finder": {}
  "selector-to-tag": {}
  "spell-check": {}
  tabs:
    usePreviewTabs: false
  "tree-view":
    hideIgnoredNames: true
  welcome:
    showOnStartup: false
".blade.html.php.text":
  editor:
    commentEnd: " --}}"
    commentStart: "{{-- "
".html.php.text":
  editor:
    tabLength: 4
lexicalunit commented 8 years ago

Interesting!

So the section:

".html.php.text":
  editor:
    tabLength: 4

would seem to indicate that your editorconfig settings are only partly being applied to Atom correctly. Missing is another section I would expect to see that would set the tabLength for json, given your editorconfig you posted previously:

[*.{json,php}]
indent_size = 4

It's possible that the editorconfig package has a bug in it related to parsing this section of your config. When I have time I'm going to try digging into the editorconfig package and see if I can figure out what's going on there. In the meantime, I believe you can get things working in Atom by adding the following lines to the end of your Atom config file:

".json.source":
  editor:
    tabLength: 4
vinkla commented 8 years ago

You've really done some digging. Thanks for that!

Maybe @sindresorhus (the atom-editorconfig maintainer) has an idea why it behaves like this?

sindresorhus commented 8 years ago

// @florianb (active atom-editorconfig maintainer)

florianb commented 8 years ago

Thanks for getting us involved @vinkla.

Assuming the json-specific configuration is set to indent_style = tab this should be a bug in atom-editorconfig, since it does not fallback to the indent_size if no tab_width is set (as the editorconfig site states at the property description of tab_width).

If setting tab_width instead of indent_size won't fix the issue with the current atom-editorconfig-release it may be that the issue belongs to the parsing of the editorconfig-file which is not being done by editorconfig-core-js.

However, It would be helpful to know if replacing indent_size by tab_with resolves this for your issue. I will file a bug in atom-editorconfig and could also stop investigating in that case.