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

Exclude Emoji from ASCII comment requirements. #5310

Closed
envygeeks opened this issue Dec 25, 2017 · 9 comments
Closed

Exclude Emoji from ASCII comment requirements. #5310

envygeeks opened this issue Dec 25, 2017 · 9 comments

Comments

@envygeeks
Copy link

I get that comments should be written in English (or ASCII,) a subset of UTF-8, but Emoji's neither get translated, or adjusted, and are often skipped and overlooked as they fall within the private space of Unicode. Which means no matter what language a comment is going to, an emoji is an emoji, a universal expression.

@envygeeks
Copy link
Author

I'm happy to look into this and possibly ship a pull request if this is accepted.

@pocke
Copy link
Collaborator

pocke commented Dec 26, 2017

Seems good to me, but I think we need a discussion. I have two issues with the proposal.

First, probably it should be optional. There may be people who do not like emoji in comments.

Second, the cop name will be inappropriate. It is a bit strange that Style/AsciiComment cop allows non-ASCII characters (emoji).
But I'm not sure what the cop name should be. Maybe it should be Style/EnglishComment, but the cop does not check English grammar, so it seems inappropriate also.

What do you think?

@envygeeks
Copy link
Author

IMO this rule should have never existed because it's particularly prone to being wrong. Take "Hola Mundo", it's Spanish, and would pass under both rule names. ASCII neither guarantees, nor promises English, and unless EnglishComment plans to actually test words to ensure they are English, it's prone to just being wrong. Here we flat out disable it because of that, and because we consider it insulting.

My offer to fix this was surfaced from a client, and to ease the pain of the community.

@deivid-rodriguez
Copy link
Contributor

I always disable this cop as well since it finds more false than true positives. I expressed my opinion in the PR introducing this cop.

@gafrom
Copy link

gafrom commented Feb 19, 2018

Sometimes comments do require emotions and it is a natural way to express those with emoji.

@pocke, will the request find its way further?

@pocke
Copy link
Collaborator

pocke commented Feb 26, 2018

Sorry for my late reply.

I think ExcludeEmoji or some option is good feature.

@bbatsov
Copy link
Collaborator

bbatsov commented Sep 15, 2018

There hasn't been much activity on this ticket and our Core Team is spread too thin on so many tasks, so I'll just close it for the sake of having a cleaner lists of tasks to focus on. We'd gladly review a PR, but it's unlikely that'd we're going to tackle this ourselves in the foreseeable future.

@bbatsov bbatsov closed this as completed Sep 15, 2018
@JoshuaKGoldberg
Copy link

@bbatsov I'd be happy to send a PR, if you're willing to reopen?

@dvandersluis
Copy link
Member

See #9674.

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

No branches or pull requests

7 participants