Navigation Menu

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

Style/AsciiComments default Enabled is true #9674

Closed
esotericpig opened this issue Apr 3, 2021 · 11 comments · Fixed by #10045
Closed

Style/AsciiComments default Enabled is true #9674

esotericpig opened this issue Apr 3, 2021 · 11 comments · Fixed by #10045

Comments

@esotericpig
Copy link

I know that you can change it to be disabled, but it seems a bit odd that Style/AsciiComments is enabled by default. I only noticed when working on a Japanese project, and I put Japanese in a comment.

I understand wanting this for identifiers, constants, etc., but why for comments?

If it's to enforce English comments, I don't think it should be on by default. It sends the message that RuboCop is, by default, for English speakers and not for the global audience.

If it's to enforce Ascii encoding for the file, that is rare nowadays (since Ruby v2), so it seems like AsciiComments should still be defaulted to disabled (since the majority of Rubyists now have source files defaulted to UTF-8).

Thanks for the consideration, and I understand if it will remain defaulted to enabled.

@marcandre
Copy link
Contributor

I agree. This also precludes emojis in comments.

@dvandersluis
Copy link
Member

dvandersluis commented Apr 3, 2021

#72 for historical context.

This cop doesn’t really enforce English (if we even should), but just languages that use the Latin alphabet. I think it’s fine to have the cop for those who want it but even an English codebase can have a need for comments that contain non ascii.

Disabled by default makes sense to me.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 3, 2021

Yeah, the idea was mostly to make sure that people stick to English in comments, with the assumption they'd likely have an international team working on the codebase. The cop was inspired partly from my own experience working in German/Austrian company where many people didn't know English yet the German and Austrian devs would often use German for identifiers and comments making it hard for people like me to understand what they meant. :-) OSS libraries are commonly documented in English as well.

I'm fine with retiring this cop, as agree it's somewhat controversial, plus detecting what's really English is pretty complex. Still, this can't happen before RuboCop 2.0. I guess we should also change the cop's name and improve its documentation.

@esotericpig
Copy link
Author

I guess in a somewhat related note the enabled-by-default Style/NumericLiterals requires numbers to be grouped by 3s by default:

100_000

But Japanese (and probably Chinese as well?), like to group it in 4s:

10_0000

If want to make it more international-friendly, maybe either turn this off by default or add a groupby option to the cop.

Actually, I ran into this when testing with Chinese phone numbers, which are grouped by 4 in mainland China:

phone = 12_3456_7890

@JoshuaKGoldberg
Copy link

👋 coming from #5310 I'm interested in sending a PR. Would the team consider accepting one?

@dvandersluis
Copy link
Member

@JoshuaKGoldberg how is this cop useful for you, aside from wanting it to accept emoji? I still think disabling it by default as discussed above makes the most sense.

We recently ended up disabling it in my work repo because it has too many edge cases (for instance, a comment containing a link with a non-ascii character, or a comment giving an example of what some code will generate which includes utf-8).

@JoshuaKGoldberg
Copy link

Exactly, I think disabling it by default makes sense.

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 26, 2021

Agreed.

@dvandersluis
Copy link
Member

dvandersluis commented Aug 26, 2021

@bbatsov are we good with doing that now? I'm going to just do it now 😁

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 26, 2021

I think yes - it's the kind of cop that no one's going to notice got disabled.

dvandersluis added a commit to dvandersluis/rubocop that referenced this issue Aug 26, 2021
dvandersluis added a commit that referenced this issue Aug 26, 2021
[Fix #9674] Disable `Style/AsciiComments` by default.
@JoshuaKGoldberg
Copy link

Glorious, thanks!

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

Successfully merging a pull request may close this issue.

5 participants