-
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] More verbs/nouns fixes in disambiguation.xml #11104
Conversation
WalkthroughThe pull request introduces a new rule in 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 (3)
languagetool-language-modules/pt/src/main/resources/org/languagetool/resource/pt/disambiguation.xml (3)
3816-3835
: LGTM! Consider adding more test cases.The rule correctly handles disambiguation of past participles when preceded by an infinitive verb and determiner. The provided examples demonstrate valid cases.
Consider adding negative test cases (examples where the rule should not apply) to ensure the pattern doesn't over-match.
Line range hint
3836-3862
: LGTM! Consider enhancing documentation.The rule correctly handles disambiguation of past participles in hyphenated words. The examples effectively demonstrate the rule's application.
Consider adding inline documentation explaining why certain exceptions are needed in the pattern, particularly:
<exception postag_regexp='yes' postag='V.+|CS|RG|CC|SPS.+|[DP].+'/>
Line range hint
3863-3899
: LGTM! Consider pattern optimization.The rule correctly handles disambiguation of verbs followed by specific words. The extensive examples effectively demonstrate various use cases.
Consider:
- Breaking down the complex pattern into multiple simpler rules for better maintainability
- Adding comments explaining why each optional element is needed
- Grouping the target words (pela|como|sobre|nada|acerca|el[ae]s?) into an entity for reuse
📜 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/resource/pt/disambiguation.xml
(1 hunks)
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/resource/pt/disambiguation.xml (1)
3816-3836
: Consider adding more test cases to verify edge cases.While the provided examples cover common cases, consider adding test cases for:
- Negated forms (e.g., "não vou terminar o doutorado")
- Complex determiners (e.g., "vou terminar aquele doutorado")
- Multiple participles (e.g., "peixe-dourado-prateado")
📜 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/resource/pt/disambiguation.xml
(1 hunks)
🔇 Additional comments (3)
languagetool-language-modules/pt/src/main/resources/org/languagetool/resource/pt/disambiguation.xml (3)
3816-3835
: LGTM! Well-structured rule for handling rare verbs in infinitive-participle contexts.
The rule effectively prevents rare verbs from being incorrectly tagged as verbs when they appear after an infinitive verb and determiner. The examples provided demonstrate valid use cases.
3836-3836
: LGTM! Well-structured rule for handling past participles in hyphenated words.
The rule correctly handles cases where past participles appear in hyphenated words, removing the verb tag when appropriate. The examples demonstrate common use cases like "não-autorizado" and "peixe-dourado".
3816-3836
: Verify the impact on existing rules.
The placement of these rules at the end of the file is correct, but let's verify there are no conflicts with existing rules.
✅ Verification successful
No conflicts found with the rule placement and pattern
The rule VERB_INFINITIVE_PASTPARTICIPLE_VERBS_RARE
is correctly placed at the end of the disambiguation rules section, and its pattern does not conflict with other rules. The search results show that:
- The rule is part of a series of carefully ordered rules that handle rare verb forms
- Similar patterns in other rules (like
NOUN_HYPHEN_PASTPARTICIPLE_VERBS_RARE
andPELA_COMO_SOBRE_NADA_ACERCA_ELAR_VERBS_RARE
) are also placed in the same section - The rule's specific pattern (VMN0000 followed by D[AI] and VMP00) is unique and doesn't overlap with other rules' patterns
- The examples provided in the comments demonstrate the intended behavior
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential rule conflicts by looking for similar patterns in existing rules
# Search for rules that also handle VMN0000, D[AI].+, and VMP00.+ patterns
rg -A 5 'VMN0000.*D\[AI\]|VMP00' | grep -v 'VERB_INFINITIVE_PASTPARTICIPLE_VERBS_RARE|NOUN_HYPHEN_PASTPARTICIPLE_VERBS_RARE'
# Search for rules that handle hyphenated words with past participles
rg -A 5 'hifen.*VMP00|VMP00.*hifen'
Length of output: 73717
Script:
#!/bin/bash
# Search for rules that handle similar patterns with VMN0000 and VMP00 in the same rule
rg -A 5 'VMN0000.*VMP00|VMP00.*VMN0000' languagetool-language-modules/pt/src/main/resources/org/languagetool/resource/pt/disambiguation.xml
# Search for rules that appear before the VERB_INFINITIVE_PASTPARTICIPLE_VERBS_RARE rule
rg -B 10 'VERB_INFINITIVE_PASTPARTICIPLE_VERBS_RARE' languagetool-language-modules/pt/src/main/resources/org/languagetool/resource/pt/disambiguation.xml
Length of output: 1112
Script:
#!/bin/bash
# Search for rules that handle verb patterns with D[AI] and VMP00 to check for potential conflicts
rg -B 5 -A 5 'postag=.D\[AI\].+.*VMP00|VMP00.*postag=.D\[AI\]' languagetool-language-modules/pt/src/main/resources/org/languagetool/resource/pt/disambiguation.xml
# Check for any rules that might be affected by this rule's position
rg -B 5 -A 15 '<rule.*>.*<pattern>.*<token postag=.VMN0000.*<token postag=.D\[AI\]' languagetool-language-modules/pt/src/main/resources/org/languagetool/resource/pt/disambiguation.xml
Length of output: 350
Script:
#!/bin/bash
# Try a more lenient search pattern to find rules with similar verb patterns
rg -B 5 -A 10 'postag=.VMN0000.*postag_regexp="yes".*<token.*postag=.D' languagetool-language-modules/pt/src/main/resources/org/languagetool/resource/pt/disambiguation.xml
# Also search for any rules that handle past participles after determiners/articles
rg -B 5 -A 10 'postag=.D.*>.*<token.*postag=.VMP00' languagetool-language-modules/pt/src/main/resources/org/languagetool/resource/pt/disambiguation.xml
# Look for any rules that might be affected by the position of this rule
rg -B 5 -A 10 '<rule.*name="Remove.*verb' languagetool-language-modules/pt/src/main/resources/org/languagetool/resource/pt/disambiguation.xml
Length of output: 7557
Another milestone!
Summary by CodeRabbit