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

Empty curly braces should not contain a space #594

Closed
glyn opened this issue Oct 30, 2013 · 7 comments
Closed

Empty curly braces should not contain a space #594

glyn opened this issue Oct 30, 2013 · 7 comments
Assignees
Labels

Comments

@glyn
Copy link

glyn commented Oct 30, 2013

I am using RuboCop 0.14.1 and it is objecting to the way RubyMine formats empty curly braces. I raised bug http://youtrack.jetbrains.com/issue/RUBY-14475 against RubyMine, but they don't intend to fix it because they believe it is working against the (implicit) spec. See http://youtrack.jetbrains.com/issue/RUBY-12659 for their rationale.

The bottom line is that:

https://github.com/bbatsov/ruby-style-guide

contains formatting which is objected to by RuboCop.

Here's an example cop message:

spec/java_buildpack/util/download_cache_spec.rb:297:56: C: Space missing inside {.
        DownloadCache.new(root).get('http://foo-uri/') {}
@bbatsov
Copy link
Collaborator

bbatsov commented Oct 30, 2013

Yeah, I consider this a bug as well. @jonas054 Will you have a look at this?

@ghost ghost assigned jonas054 Oct 30, 2013
@jonas054
Copy link
Collaborator

Space again. I'm on it!

@jonas054
Copy link
Collaborator

It's a little less straight-forward than I imagined. There are some choices to make.

We could take the easy way out and just allow empty block braces with or without space inside. This is how SpaceInsideHashLiteralBraces works today.

But that's not how we usually do things around here. We tend to enforce one style and make it configurable if needed. I've been working on a solution along those lines, adding to SpaceAroundBlockBraces an EnforcedStyleForEmptyBraces parameter with values space and no_space. But then SpaceInsideHashLiteralBraces needs to go the same way, I think. For consistency. The problem there is that it has a parameter called EnforcedStyleIsWithSpaces with values true and false. It would need to be changed to EnforcedStyle with values space and no_space. Again, for consistency. Then we break backwards compatibility in that cop.

@bbatsov @yujinakayama Your thoughts?

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 1, 2013

@jonas054 I'm OK with that change. The impact on users would be minimal and since we're pre-1.0 we're allowed to do things like this.

@glyn
Copy link
Author

glyn commented Nov 4, 2013

Thanks. Which release/milestone will this fix appear in?

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 4, 2013

In will appear in 0.15.0, which I hope to release around midweek.

@glyn
Copy link
Author

glyn commented Nov 4, 2013

Perfect. Thanks very much!

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

No branches or pull requests

3 participants