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

Feature request: RSpec/ContextWording supports Japanese #745

Closed
pocke opened this issue Feb 27, 2019 · 4 comments
Closed

Feature request: RSpec/ContextWording supports Japanese #745

pocke opened this issue Feb 27, 2019 · 4 comments

Comments

@pocke
Copy link
Contributor

pocke commented Feb 27, 2019

Problem

RSpec/ContextWording does not work well to check Japanese.
It has two problems.

First, usually we use a suffix to specify "when". For example, when a user exists is translated to ユーザーが存在する時. means when.
But the cop only supports prefix, so we cannot configure it for Japanese.

Second, Japanese does not separate words with space. For example, ユーザー が 存在する 時 is not appropriate as Japanese.
But the cop split the sentence with space.
https://github.com/rubocop-hq/rubocop-rspec/blob/0442757fb4af570994b37b3e0ca35baeec635eb0/lib/rubocop/cop/rspec/context_wording.rb#L45
So If the cop supports suffix, it does not enough.

Solution Ideas

I have two ides.

First, adding Suffixes and Split options to the cop.
For example:

RSpec/ContextWording:
  Suffixes:
    - ''
  Split: false # default: true

The cop splits context sentence only if Split option is enabled. If it is disabled, the cop just checks with start_with or end_wtih method.

Second, adding Patterns option to the cop.
For example

RSpec/ContextWording:
  Patterns:
    - "^when"
    - "^if"
    # Suffix pattern for japanese
    - "時$"

I like the first idea. The second idea is more powerful, but I think we do not need regexp in most cases.


The original idea is issued in rubocop-hq/rubocop-jp by @kyohah. rubocop/rubocop-jp#49
Thank you!

@pirj
Copy link
Member

pirj commented May 9, 2019

I remember we decided to split as opposed to using start_with? due to "whenever" that would also match the /^when/ pattern.

The logic of composition of Prefixes with Suffixes is not obvious. While reading the configuration, it would be hard to understand if it is "one of the Prefixes and one of the Suffixes", or "one of the Prefixes or one of the Suffixes".

WDYT of failing the cop when both Suffixes and Prefixes are defined in the configuration?

@dgollahon
Copy link
Contributor

I think the point about composing Suffixes and Prefixes is a good one--it very well might be confusing to have both. I don't know if we need regexes or not, but at least that is relatively clear with a list of patterns. The pattern still be easily used with a , \b, or \s appended to each item, but if a user wants to customize the list they are might forget to do that.

The other issue in composition is you need to be able to make any/all prefixes valid options. If, in japanese or another language, you only care about the suffix, then you can't whitelist all prefixes using the existing configuration since it could be any character / word.

Re: Splitting, another option that might be worth considering is specifying a delimiter (' ' for us, '' would split on each character, but i guess that wouldn't work if you wanted to read multiple characters).

Regex might be overkill, but it seems like it might be simpler than the Suffixes/Prefixes/Split combinations.

@ydah
Copy link
Member

ydah commented Jan 23, 2023

I overlooked this Issue, but I think this Issue is resolved in #1358 because it has the same as #745 .

@pirj
Copy link
Member

pirj commented Jan 23, 2023

Thank you, @ydah for triaging this!

Fixed in #1358

@pirj pirj closed this as completed Jan 23, 2023
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

4 participants