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

Which options should Unibeautify support (if not all of them) #1

Open
stevenzeck opened this issue Jan 25, 2018 · 5 comments
Open

Which options should Unibeautify support (if not all of them) #1

stevenzeck opened this issue Jan 25, 2018 · 5 comments
Assignees
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@stevenzeck
Copy link
Contributor

ESLint has a ton of rules, and even though maybe 1/3 can be fixed/formatted using it's API, that still leaves 90-95 rules that can be. You can see which rules can be fixed on this page, those with a wrench in the second column: https://eslint.org/docs/rules/.

All of those options have been "added" to the Beautifier here, but they still need to be added to Unibeautify's options and implemented properly here in the beautifier. I've managed to match up 10 with the options already there, so that leaves roughly 80 that need to be added (options.ts will have a log of lines to it in the end).

Do we want to support all of the options/rules? If not, what do we want to keep?

@stevenzeck stevenzeck added the question Further information is requested label Jan 25, 2018
@Glavin001 Glavin001 added the help wanted Extra attention is needed label Jan 25, 2018
@Glavin001
Copy link
Member

Great work thus far!!

Ideally, we do support all of them. However, we should take a look at other linters to determine the best/most popular name for these options. Also, how are we going to deal with options which support being an object with additional configuration properties 😕 .

For example: https://eslint.org/docs/rules/prefer-const

{
    "prefer-const": ["error", {
        "destructuring": "any",
        "ignoreReadBeforeAssign": false
    }]
}

Another problem with these more complex options is how we are going to deal with importing ESLint configuration files, etc. See https://github.com/Unibeautify/unibeautify-options-importer
Unibeautify may read an ESLint config file, simplify the complex option, and then pass the simplified option back to ESLint, while loosing part of the configuration 👎 . I'm not sure how we should get around this. We should figure this part out before we start adding ESLint options.

@Glavin001
Copy link
Member

Fortunately, we can handle some of these complex options already: https://eslint.org/docs/rules/no-confusing-arrow

{
    "rules": {
        "no-confusing-arrow": ["error", {"allowParens": false}]
    }
}

This would be a Unibeautify option requiring two option dependencies no-confusing-arrow (not available yet) and allowParens (AKA allow_parens from Unibeautify). This will actually be a nicer experience for users, to define these pieces separately and generate a combined ESLint file. 👍

@stevenzeck
Copy link
Contributor Author

In figuring out which options to use, I would suggest the going through following (in order):

  1. Look for an ESLint config file in the project directory. I don't know if ESLint would do this automatically (see useEslintrc under https://eslint.org/docs/developer-guide/nodejs-api#cliengine) or if we have to specify it (seeconfigFile under same link). It does support cascading, so if you have one config in one directory and another config in it's parent, any file in the subdirectory would read it's config file instead of the parent. If there is no config in the subdirectory, then it looks in the parent (and so on).

  2. Use Unibeautify's config file. I don't know if it supports cascading, if so, follow the same thing as above.

  3. If neither exist, use the values set by the user in the options UI for whatever editor they are using.

@stevenzeck
Copy link
Contributor Author

For the complex options, I see two choices if we want to support them fully outside of an ESLint specific configuration file. Take this example:
https://eslint.org/docs/rules/eqeqeq. The base options are always and smart. But under always there are further options on handling null literals, those being to never use eqeqeq against null or to ignore and not apply the rule to them.

One option is to have an "option within an option". So when always is used, the user can opt-in on how they want to handle nulls. The second option is to have each sub-option from ESLint's point of view and make it its own option. So in this case it would be always, always - except on null, always - ignore null, and `smart.

@muuvmuuv
Copy link
Contributor

@szeck87 I would add those steps:

....

  1. Use Unibeautify's global config (so we could say a .unibeautifyrc in the users' root e.g. ~/.unibeautifyrc.yml)

  2. Use .editorconfig

  3. Use the rules set within the Editor

  4. Let the beautifier choose the config if it is possible (ESLint supports this: https://eslint.org/docs/developer-guide/nodejs-api#clienginegetconfigforfile)

  5. throw new Error("Please create a minimal .unibeautifyrc.yml configuration file in your project to get Unibeautify working.")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants