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.

Sign up for GitHub

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

php-cs-fixer allow risky option #1614

Merged
merged 7 commits into from
Apr 21, 2017
Merged

php-cs-fixer allow risky option #1614

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
93ac2a0
added allow-risky
JonidBendo Apr 20, 2017
b57e5ac
Update php-cs-fixer.coffee
JonidBendo Apr 20, 2017
907ca37
added option
JonidBendo Apr 20, 2017
3655f2e
Update options.json
JonidBendo Apr 20, 2017
6f2354d
added option to language
JonidBendo Apr 21, 2017
267082d
autogenerated files
JonidBendo Apr 21, 2017
449d95b
added v2-clarification in desc
JonidBendo Apr 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/beautifiers/php-cs-fixer.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ module.exports = class PHPCSFixer extends Beautifier
"fix"
"--rules=#{options.rules}" if options.rules
"--config=#{configFile}" if configFile
"--allow-risky=#{options.allow_risky}" if options.allow_risky
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we will always pass in allow_risky?

php-cs-fixer --allow-risky=yes
php-cs-fixer --allow-risky=no

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Glavin001 I thought the if there is not passing this option as long as the allow_risky is not changed (not in the configuration file), after changed yes.

I know for sure that if not defined its set to no by default. So the general behaviour would be.

  1. As long as it is not in the config file it will not be included (I am not sure about this)
  2. If its set to no it will be included but nothing will change from the default behaviour
  3. If it is set to yes then rules like symfony and so on (that are considered risky) may be applied

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! I think this is good as is.

As long as it is not in the config file it will not be included (I am not sure about this)

The default value should be present, so no. So as long as php-cs-fixer --allow-risky=no is valid, then we are all good.

"--using-cache=no"
]
if version is 1
Expand Down
6 changes: 6 additions & 0 deletions src/languages/php.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ module.exports = {
type: 'string'
default: ""
description: "Add rule(s). i.e. line_ending,-full_opening_tag,@PSR2 (PHP-CS-Fixer 2 only)"
allow_risky:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Don't forget to regenerate the documentation with npm run docs.

See https://github.com/Glavin001/atom-beautify/blob/master/docs/add-languages-and-beautifiers.md#options-documentation for details.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have probably seen this documentation from start 👍 . Well everything should be ready now.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

title: "Allow risky rules"
type: 'string'
default: "no"
enum: ["no", "yes"]
description: "allow risky rules to be applied!"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add (PHP-CS-Fixer 2 only)? I should have noticed this sooner, sorry about that.

phpcbf_path:
title: "PHPCBF Path"
type: 'string'
Expand Down
20 changes: 19 additions & 1 deletion src/options.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5852,6 +5852,24 @@
"namespace": "php"
}
},
"allow_risky": {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be automatically generated. I do not see a change to the PHP language adding allow_risky option?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Glavin001 I haven't really worked with modules before, this was my first time and I did not know about the autogeneration part, but i got it now, I also committed the change to the language file.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem! Getting closer.

"title": "Allow risky rules",
"type": "string",
"default": "no",
"enum": [
"no",
"yes"
],
"beautifiers": [
"PHP-CS-Fixer"
],
"key": "allow_risky",
"language": {
"name": "PHP",
"namespace": "php"
},
"description": "allow risky rules to be applied!"
},
"phpcbf_path": {
"title": "PHPCBF Path",
"type": "string",
Expand Down Expand Up @@ -9040,4 +9058,4 @@
}
}
}
}
}