Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add editorConfig properties to package.json? #30

Open
kevinSuttle opened this issue Dec 8, 2015 · 40 comments
Open

Add editorConfig properties to package.json? #30

kevinSuttle opened this issue Dec 8, 2015 · 40 comments

Comments

@kevinSuttle
Copy link

In the spirit of ESLint and Babel, would it make sense to allow editorConfig settings in package.json?
I feel like it probably doesn't, but I wanted to have it documented either way to be definitive.

i.e.

"editorConfig" : {
}

Could also add presets: #31

@kevinSuttle kevinSuttle changed the title Add editorConfig preset to package.json? Add editorConfig properties to package.json? Dec 8, 2015
@jednano
Copy link
Member

jednano commented Dec 11, 2015

That's rather JavaScript specific, which isn't really in the design goals of EditorConfig.

@kevinSuttle
Copy link
Author

@jednano
Copy link
Member

jednano commented Dec 11, 2015

Ah, see, I didn't realize this was on the core-js repo. Seeing as it is, I have no problems with such a feature; though, we still need to look in subfolders for both .editorconfig and package.json files, which means the package.json file needs to define "root": true if that's the case. Then, what happens if you have both an .editorconfig as well as a package.json file in the same folder, both with an EditorConfig configuration? Which one takes precedent? I can see some issues to address here.

@kevinSuttle
Copy link
Author

@corysimmons
Copy link

Would prefer to see someone make some standard config file (or folder) for all configs to live in rather than continuing to clutter package.json with xyz.

...that said, if it does get in, I'll probably use it.

@jonathantneal
Copy link

Likewise, I will use this. @jedmao, what can we do to help? Do we need to look into the plugin and make a PR for the functionality? Could someone help with documentation? I’d love to see this get in on the next release.

@jednano
Copy link
Member

jednano commented Feb 26, 2016

First, we need to come up with an acceptable spec. At first glance, I'm imagining something like the following:

{
  "editorConfig": {
    "root": true,
    "settings": [
      {
        "*": {
          "indent_style": "space",
          "indent_size": 4
        }
      },
      {
        "package.json": {
          "indent_size": 2
        }
      }
    ]
  }
}

It doesn't look as clean as the INI file format that .editorconfig uses, but this is the only way I could imagine supporting it in a package.json file.

BTW – you should theoretically be able to have any number of package.json files throughout your project. I would imagine that if both package.json and .editorconfig files were in a directory that the .editorconfig file would take precedence, but I'm open to discussion.

I took a look at the multilevel-ini and other INI parsing packages, but none of them account for ordered settings. I think the example structure I provided above is the only structure that would support this.

@jednano
Copy link
Member

jednano commented Feb 26, 2016

Another alternative would be the following structure:

{
  "editorConfig": [
    ["root", true],
    ["*", {
      "indent_style": "space",
      "indent_size": 4
    }]
  ]
}

@jonathantneal
Copy link

@jedmao, thanks for the fast reply!

I’m a fan of the first syntax you’ve shared, and I would expect, as you’ve said, that the .editorconfig file takes precedent when both it and a package.json are present. Would this support an extends property being added alongside root and the other rules?

@jednano
Copy link
Member

jednano commented Feb 26, 2016

@jonathantneal anything above existing functionality would be a separate ticket. Please open another issue for that feature request and detail how it might work.

BTW – I'm not at all suggesting I have any bandwidth to implement this package.json feature. I still have zero interest in the feature for my own use, so I won't be using it.

I would, however, consider merging in a PR, if it were implemented properly. That is, of course, the purpose of this discussion, to talk about what a proper implementation would be.

@kevinSuttle
Copy link
Author

@jonathantneal
Copy link

jonathantneal commented Sep 16, 2016

Hey there. What can I do to help with moving this forward? Do we still need help defining the request more clearly? We have a year of sharable configs under our belt for most if-not-all linters. Does the configuration just need an outlined JSON version? Is this something we should be writing plugins for Atom / Sublime first for?

Example package.json with inline rules:

{
    "root": true,
    "rules": {
        "*": {
            "indent_style": "tab",
            "indent_size": "tab",
            "tab_width": "4",
            "end_of_line": "lf",
            "charset": "utf-8",
            "trim_trailing_whitespace": true,
            "insert_final_newline": true,
            "max_line_length": "off"
        }
    }
}

Example package.json extending a local editor-config-jonathantneal package:

{
    "root": true,
    "extends": "jonathantneal"
}

@ButuzGOL
Copy link

ButuzGOL commented Dec 5, 2016

+1

@andreiglingeanu
Copy link

I would love to contribute somehow on getting this thing moving! There's really no other way to re-use .editorconfig's across multiple repos.

@jednano
Copy link
Member

jednano commented Apr 10, 2017

Nope. No other way. And remember, this is just for the js core. Other cores would need some kind of implementation too. I think we should stay focused on the OP's request and just support configuration in a package.json file for now and then start worrying about how the "extends" feature would work.

@andreiglingeanu
Copy link

@jedmao Every core has its own options parsing code duplicated? That is, .editorconfig is pretty much hardcoded in each of them, right?

@jednano
Copy link
Member

jednano commented Apr 10, 2017

Yes, it has to be duplicated because the cores are written in different languages. The only thing that's not duplicated are the tests, which are shared across cores.

@andreiglingeanu
Copy link

Seems reasonable enough, thanks.

@jednano
Copy link
Member

jednano commented Oct 21, 2017

I just did a pretty hefty rewrite of the source code into TypeScript. Anyone interested in taking this feature further? Been a bit of silence.

@andreiglingeanu
Copy link

andreiglingeanu commented Oct 23, 2017

I'd love to hop in to help a bit but I'm not really able to get the local dev setup working. The local bin/editorconfig definitely doesn't get created after doing npm install; npm link. Is that something easily fixable?

Other than that, the implementation for supporting editorConfig entry in package.json seems quite doable to me.

@jednano
Copy link
Member

jednano commented Oct 23, 2017

@andreiglingeanu I think the issue you are experiencing is related to #55. Perhaps we should do it the other way and remove the ./bin folder from package.json and restore the ./bin/editorconfig.

@andreiglingeanu
Copy link

Hey @jedmao, yes, that it is. The bin/editorconfig file did not get generated at all in my local setup.

@jednano
Copy link
Member

jednano commented Oct 24, 2017

@andreiglingeanu, aha! See #60. Now that the package.json file expects to be published in the ./dist folder, that's where the npm link command needs to be run. Make sense? Or npm link ./dist from the project root. Either way.

@andreiglingeanu
Copy link

We are good to go! Expect the pull request soon.

@BtM909
Copy link

BtM909 commented Feb 19, 2018

Maybe I'm late to the party, but would this adhere to the cosmiconfig configuration file spec?

One of the main reasons to avoid a mandatory "root": true would be that it

Stops at the first configuration found, instead of finding all that can be found up the directory tree and merging them automatically.

Secondly, it would make it easier to adapt the other (defacto) standard to support different types of configuration files.

Thirdly, priority of configuration files can also be configured (and therefore predictable).

Would Editorconfig be open for other configuration file formats? Or would it be easier to open a separate issue to discuss that?

@jednano
Copy link
Member

jednano commented Feb 20, 2018

@BtM909 I was looking at package-json myself, but it looks like cosmiconfig has a bit more popularity. Not sure the delta between them.

As for other configuration file formats. What did you have in mind?

@BtM909
Copy link

BtM909 commented Jun 25, 2018

Sorry, completely missed your update... My apologies!

As for other configuration file formats. What did you have in mind?

What formats are you specifically referring to? I was just references the fact that cosmiconfig supports multiple file formats by default, like:

  • a package.json property
  • a JSON or YAML, extensionless "rc file"
  • an "rc file" with the extensions .json, .yaml, .yml, or .js.
  • a .config.js CommonJS module

@mattgreenfield
Copy link

@andreiglingeanu - did you ever make the pull request for this? Thanks for your help.

@ColtHands
Copy link

Is there any movement on this feature?

jrnewton added a commit to jrnewton/udemy-vue-complete-guide that referenced this issue Jan 7, 2021
jrnewton added a commit to jrnewton/udemy-vue-complete-guide that referenced this issue Jan 7, 2021
@lucasvazq
Copy link

bump

@SrBrahma
Copy link

Up! I hate small files cluttering my workspace. It could really be in the package.json.

@ColtHands
Copy link

ColtHands commented Jun 16, 2022

@lucasvazq @SrBrahma

Its not hard to implement this feature, but the main problem is that this repo is long dead, it uses really old versions and heck even tslint which has been long dead as well.

Even if the code itself would be easy to implement, I'd be stuck with version bumping, security issues and manual version testing to make sure it all works.

Not even talking about the fact that the last pull requests were just version bumps and that activity dates to 2019.

If you like the package you could fork it and create your own version.

But i hightly suggest you use something like

prettier or eslint for your tabs and spaces

for end of line you could just use your editors features or just edit it directly with git
git config --global --edit
and set your end of line there

@hildjj
Copy link
Collaborator

hildjj commented Oct 10, 2022

OK, we're back up and running. I'm looking through open issues before doing a release. Does anyone still want this?

@jrnewton
Copy link

Does anyone still want this?

Yes please!

@hildjj
Copy link
Collaborator

hildjj commented Oct 11, 2022

OK, if this moves forward here is the proposed approach. For each package.json in this directory or any parent (as long as root hasn't happened), read that file, parse it as JSON, and look for a root key of editorconfig, which will have this shape:

{
  "global": { "root": true, "otherProperty": 12 },
  "rules": {
    "*": { "indent_style": "tab" }
  }
}

This gets turned into a ParseStringResult by flattening slightly. Using ParseStringResult directly was just a little too user-unfriendly. We need to be able to handle other top-level properties in the global section, so that if we want to add something like extends later, we can. I don't want to reserve any other words in that namespace other than root at the moment. This approach also means that we can't have two rule sections that have the same pattern, but that seems like a good restriction anyway.

If both .editorconfig and package.json are found in the same directory, the package.json file is processed first, so that the values from .editorconfig will overwrite those in package.json, as long as package.json does NOT define "global": { "root": true }, which will stop processing before the .editorconfig file is read.

Note that many tools in the ecosystem will not be using the JS core, so any rules defined in package.json will not be recognized by those tools until the other cores are updated. First step there would be to add tests to the test suite, I think. Given that, we should probably raise this up to the larger group to have opinions on the syntax. Given the above, if we do this, it will come after the pending release.

Now is the time to bikeshed all of these details.

@jonathantneal
Copy link

Likewise, I will use this.

@HolgerJeromin
Copy link

This approach also means that we can't have two rule sections that have the same pattern, but that seems like a good restriction anyway.

We also have no rule order. I am not sure if this is important but I have this scenario in mind:

{
  "*": {"optionA": true},
  "subfolder/*": {"optionA": false}
}

@hildjj
Copy link
Collaborator

hildjj commented Oct 11, 2022

Given that order needs to matter, ParseStringResult is probably the correct type. Its typescript definition is:

export type SectionName = string | null
export interface SectionBody { [key: string]: string }
export type ParseStringResult = [SectionName, SectionBody][]

@Lioness100
Copy link

Is there any way shareable configs could be added to this? (like: "editorConfig" : { "extends": ["some-package", "./local.json"] })

@hildjj
Copy link
Collaborator

hildjj commented Feb 3, 2023

Shareable configs are a separate feature request that needs to change the specification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests