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

[pt] Add logic for Brazilian municipality/state mapping #9946

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

p-goulart
Copy link
Collaborator

The core of the rule is now a regexp, and we just call the rule filter to make sure we're dealing with an actual city.

This rule doesn't check if the city actually belongs in that state – that'll be a different rule. The idea here is that only things that could be conceivably thought of as Brazilian cities should trigger the state abbreviation separation rule.

We still need to improve (or apply a similar logic?) to the US states rule.

if (suggestion.equals(underlined)) {
return null;
}
BrazilianToponymMap map = new BrazilianToponymMap();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to make this a static constant? We maybe don't want to re-initialize the object every time there's a potential match.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should definitely do that. I was certain I had, but I think I moved it around a bit and forgot to put it back.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@p-goulart p-goulart merged commit 8c88795 into master Dec 19, 2023
2 checks passed
@p-goulart p-goulart deleted the pt/grammar/city_list branch December 19, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants