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

We want to make the output match the yml expectations #688

Conversation

krainboltgreene
Copy link

I want this change so that if I need to modify a cop I can use
—show-cop, find the right one, and then copy/paste the body into the
yml and do no extra work to format it for YAML documents.

You'll notice the failing test. I attempted to fix it but the test is really clunky and apparently iterates through a series of expectations. One of these expectations fails in a way that doesn't make sense (expecting the wrong cop description).

I want this change so that if I need to modify a cop I can use
—show-cop, find the right one, and then copy/paste the body into the
yml and do no extra work to format it for YAML documents.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) when pulling 62c1d04 on krainboltgreene:improving-show-cop-output into 0d7161c on bbatsov:master.

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 4, 2014

Makes sense to output the info in yml format. @jonas054 Would you have a look the problematic test?

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 4, 2014

Btw, @jonas054, seems to me it might be a good idea for us to add --show-cop Cop option as well for people who want to see information just for a particular cop. Or maybe we can adjust -show-cops to receive a list of cop names as params - say --show-cops Tab StringLiteral MethodName. With no arguments we'd keep the existing behaviour.

@ghost ghost assigned jonas054 Jan 4, 2014
@jonas054
Copy link
Collaborator

jonas054 commented Jan 4, 2014

Yes, I'll look at this!

@jonas054
Copy link
Collaborator

jonas054 commented Jan 5, 2014

If we want to make it possible to copy and paste --show-cops output into a configuration file, we should make sure the output is YAML. Today we have

 - CollectionMethods
    - Description: Preferred collection methods.
    - Enabled: true
    - PreferredMethods: {"collect"=>"map", "collect!"=>"map!", "inject"=>"reduce", "detect"=>"find", "find_all"=>"select"}
    - SupportsAutoCorrection: true

The commit in this PR changes it to:

CollectionMethods:
  Description: Preferred collection methods.
  Enabled: true
  PreferredMethods: {"collect"=>"map", "collect!"=>"map!", "inject"=>"reduce", "detect"=>"find", "find_all"=>"select"}
  SupportsAutoCorrection: true

This is still not YAML. The values are on Ruby literal format. I propose to change the output to something very similiar to what we have in our config files:

CollectionMethods:
  Description: Preferred collection methods.
  Enabled: true
  PreferredMethods:
    collect: map
    collect!: map!
    inject: reduce
    detect: find
    find_all: select
  SupportsAutoCorrection: true

One other option is to make a more compact format where all values are output as JSON (which is a subset of YAML). This is a middle of the road solution that doesn't make anybody happy, IMO.

CollectionMethods:
  Description: "Preferred collection methods."
  Enabled: true
  PreferredMethods: {"collect":"map","collect!":"map!","inject":"reduce","detect":"find","find_all":"select"}
  SupportsAutoCorrection: true

@krainboltgreene @bbatsov @yujinakayama What do yo think?

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 5, 2014

I think it would be best if we output YAML to match the actual configuration format.

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

Successfully merging this pull request may close these issues.

4 participants