-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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] Removed "temp_off" from rule ID:COLOCACAO_PRONOMINAL_EM_O_SENDO and ID:COLOCACAO_PRONOMINAL_COM_ATRATOR_COMPLEXO #11111
Conversation
…and ID:COLOCACAO_PRONOMINAL_COM_ATRATOR_COMPLEXO
WalkthroughThe changes in this pull request involve the modification of two grammar rules in the Portuguese language module of LanguageTool. Specifically, the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
languagetool-language-modules/pt/src/main/resources/org/languagetool/rules/pt/grammar.xml (1)
Line range hint
12893-12928
: Consider documenting these grammar rule changesBoth rules being enabled by default represent a significant change in Portuguese grammar checking behavior. Consider:
- Updating the Portuguese grammar documentation to reflect these newly activated rules
- Adding a changelog entry to inform users about the stricter pronoun placement checking
- Monitoring user feedback after deployment to ensure these rules don't generate false positives
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
languagetool-language-modules/pt/src/main/resources/org/languagetool/rules/pt/grammar.xml
(2 hunks)
🔇 Additional comments (2)
languagetool-language-modules/pt/src/main/resources/org/languagetool/rules/pt/grammar.xml (2)
Line range hint 12924-12928
: Verify the impact of enabling this rule group by default
The removal of default="temp_off"
from this rule group means it will now actively enforce proclisis in constructions with "em o sendo". While this change is intentional per the PR objectives, we should verify its readiness for default activation.
Let's check for any related discussions and test coverage:
✅ Verification successful
Let me check the test cases for this rule to verify its readiness.
Let me check the complete rule definition and its examples to better understand its implementation.
Rule is well-implemented and ready for default activation
The rule is properly implemented with:
- Clear pattern matching for "em" + optional preposition + gerund constructions
- Proper filter class (PortugueseProclisisFilter) for handling suggestions
- Well-defined message explaining the grammar rule
- Multiple test examples demonstrating correct and incorrect usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for discussions and test coverage for this rule
# Search for issues/PRs mentioning this rule
gh search issues "COLOCACAO_PRONOMINAL_EM_O_SENDO" --repo languagetool-org/languagetool
# Check for test cases
rg -l "COLOCACAO_PRONOMINAL_EM_O_SENDO" --type xml
Length of output: 228
Script:
#!/bin/bash
# Search for test cases in the XML files
rg -B 5 -A 5 "COLOCACAO_PRONOMINAL_EM_O_SENDO" --type xml
# Check if there are any unit tests for this rule
fd ".*Test.*\.java" languagetool-language-modules/pt/src/test/ --exec rg -l "COLOCACAO_PRONOMINAL_EM_O_SENDO" {}
Length of output: 1597
Script:
#!/bin/bash
# Get the complete rule definition including examples
rg -B 2 -A 15 'COLOCACAO_PRONOMINAL_EM_O_SENDO.*name=' --type xml languagetool-language-modules/pt/src/main/resources/org/languagetool/rules/pt/grammar.xml
Length of output: 1120
Line range hint 12893-12897
: Verify the impact of enabling this rule by default
The removal of default="temp_off"
means this rule for pronoun placement with complex attractors will now be active by default. While the change aligns with the PR objectives, we should ensure this won't unexpectedly affect users.
Let's check for any related discussions or issues about this rule:
Removed "temp_off" from rules.
Summary by CodeRabbit