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

Allow ExtraSpacing for comments? #6383

Closed
davearonson opened this issue Oct 12, 2018 · 6 comments
Closed

Allow ExtraSpacing for comments? #6383

davearonson opened this issue Oct 12, 2018 · 6 comments

Comments

@davearonson
Copy link
Contributor

davearonson commented Oct 12, 2018

I often use a trailing comment, separated from the code by two spaces. For instance, in Ruby (I do this in many other languages as well):

    frobber.frob(widget)  # must be done before widget is munged

I find that the little bit of extra spacing helps me ignore it when reading just the code (like poring over code I wrote and trying to trace through the logic), but is not so much that it makes it hard to notice (when I'm reading over someone else's, or vice-versa, for the first time). So, I'd prefer to keep doing that, rather than knock it down to one space.

However, it triggers the ExtraSpacing cop, resulting in a lot of reports I don't really care about. I could simply disable it, but I want to keep it on for other purposes.

What I would like to see is an option to allow extra spacing for trailing comments, something like AllowForTrailingComments, similar to the existing AllowForAlignment. This can default to false, for compatibility with existing behavior.

Alternately you can try to convince me my style is so horrible it should not even be allowed. :-)

I realize this is not a very common style, so unless you convince me to dump it, I'll try to find some time (especially this month, Hacktober) to look into how Rubocop works and Make It So myself. :-)

Output of rubocop -V, IAW with the issue reporting guidelines:
0.58.2 (using Parser 2.5.1.2, running on ruby 2.3.0 x86_64-darwin14)

@mikegee
Copy link
Contributor

mikegee commented Oct 13, 2018

Rubocop tends to allow configuration for any reasonable style of code people are writing. (Perhaps even the unreasonable styles 🤔). It's the default configuration that is more hotly debated.

Your two spaces there seem fine to me. I bet a PR introducing a config to allow that style would be accepted.

Whether these extra spaces would be allowed by default is an interesting question. I'm OK with it being allowed by default. 👍

@davearonson
Copy link
Contributor Author

I've got a PR almost ready, but the contrib guidelines say to run Rubocop on my code, and there's one item that is triggering a cop but I don't see how. The code (in the spec) is:

    let(:cop_config) do
      { 'AllowForAlignment'        => allow_alignment,
        'AllowForTrailingComments' => allow_comments }
    end

and RC is saying "Layout/AlignHash: Align the elements of a hash literal if they span more than one line." They sure look aligned to me. Is this some usage of the word "aligned", with which I was not formerly familiar?

Meanwhile, I'm about to make the PR.

@mikegee
Copy link
Contributor

mikegee commented Oct 13, 2018

Maybe autocorrect that code to see what Rubocop expects?

@davearonson
Copy link
Contributor Author

davearonson commented Oct 15, 2018

D'oh, I forgot that was even an option, I generally ignore "automagically fix it for me" things as I don't trust them. ;-) I'll go try that and see.

@davearonson
Copy link
Contributor Author

It "autocorrected" it to:

    let(:cop_config) do
      { 'AllowForAlignment' => allow_alignment,
        'AllowForTrailingComments' => allow_comments }
    end

which I don't think is aligned!

It does this regardless of whether I put the braces on their own lines, or as shown. I'm going to put it back to actually aligned.

Maybe I screwed something up. Can you extract that into a file and see if RC gritches at it for you?

@davearonson
Copy link
Contributor Author

davearonson commented Oct 15, 2018

Dagnabit, RuboCop flagging it is making it fail Circle CI. Makes sense to dogfood it like that, so I suppose I can't gritch too much. ;-)

Further digging reveals that the .rubocop.yml used for the project doesn't give a value for Layout/AlignHash's EnforcedHashRocketStyle, which means it's defaulting to key, which aligns by key and forbids then aligning the hashrockets. The option table would do what I'm trying to do. But adding that and then running RuboCop is showing bazillions of offenses. So, I'm going to just go with the project's existing style.

davearonson added a commit to davearonson/rubocop that referenced this issue Oct 28, 2018
Adds the AllowBeforeTrailingComments option on Layout/ExtraSpacing cop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants