-
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
Cb changes 20241106 #10998
Cb changes 20241106 #10998
Conversation
WalkthroughThe changes in this pull request primarily involve the addition of a significant number of scientific names for various species, as well as proper nouns, including names of people, places, and products. These modifications aim to expand the vocabulary and knowledge base of the language model. Additionally, several files related to German and English language modules have been updated to include new terms, spelling corrections, and grammatical rules, enhancing the overall functionality of the language processing tool. Changes
Possibly related PRs
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: 1
🧹 Outside diff range and nitpick comments (7)
languagetool-language-modules/de/src/main/resources/org/languagetool/rules/de/grammar.xml (5)
17983-17983
: Document the temporary state of this rule group.The rule group is currently disabled by default (
temp_off
). Consider adding a comment explaining when this temporary state will be lifted and what conditions need to be met before enabling it by default.
17984-17992
: Consider adding more test cases.The rule implementation looks good with proper pattern matching and non-breaking space in the suggestion. Consider adding test cases for:
- Lower case variations (e.g., "dina4")
- Numbers other than 4 (e.g., "DinA3", "DinA5")
18001-18010
: Standardize error message format.The implementation looks good, but consider rephrasing the error message "Hier scheint ein oder zwei Bindestrich(e) zu fehlen" to be more specific about exactly how many hyphens are missing in each case, similar to the fourth rule's message.
18011-18020
: Add negative test cases.While the rule correctly handles the positive case, consider adding negative test cases (marked with
<example>...</example>
without correction) to ensure the rule doesn't trigger on valid constructs.
18021-18030
: Make the error message more specific.The current message "Bitte die Schreibweise dieses Nomens prüfen" is vague. Consider making it more specific about what aspect of the writing needs to be checked (e.g., "DIN sollte in Großbuchstaben geschrieben werden").
languagetool-language-modules/en/src/main/resources/org/languagetool/resource/en/hunspell/spelling.txt (1)
Standardize to "biodegradable" form without hyphenation
Based on the search results:
- The non-hyphenated form "biodegradable" is used in Spanish (es) and Galician (gl) dictionaries
- The English dictionary currently has both forms, which can lead to inconsistency
- The compounds.txt file uses the hyphenated form with a question mark, suggesting it's a tentative entry
Recommend:
- Remove "bio-degradable" and "bio-degradables"
- Keep "biodegradable" and "biodegradables" as the standard forms
- Update compounds.txt to remove or update the "bio-degradable?" entry
This aligns with the most common usage across languages and modern scientific writing.
🔗 Analysis chain
Line range hint
537-544
: Consider standardizing hyphenation in related terms.While both hyphenated and non-hyphenated forms are technically correct, consider standardizing the usage for consistency:
- "bio-degradable"/"bio-degradables" vs "biodegradable"/"biodegradables"
Choose one style and apply it consistently across the codebase.
Let's check for consistency in other files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of these terms to ensure consistent hyphenation rg -i "bio.?degradable"Length of output: 1419
languagetool-language-modules/en/src/main/resources/org/languagetool/rules/en/diacritics.txt (1)
Line range hint
1369-1427
: Consider grouping related entries together.While the additions are correct, consider organizing related entries together. For example:
- Group entries by language origin (Spanish, Portuguese, Turkish, etc.)
- Group entries by type (person names, place names, etc.)
This would make future maintenance and updates easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
languagetool-core/src/main/resources/org/languagetool/resource/spelling_global.txt
(1 hunks)languagetool-language-modules/de/src/main/resources/org/languagetool/resource/de/hunspell/ignore.txt
(1 hunks)languagetool-language-modules/de/src/main/resources/org/languagetool/resource/de/hunspell/spelling.txt
(1 hunks)languagetool-language-modules/de/src/main/resources/org/languagetool/resource/de/multitoken-ignore.txt
(1 hunks)languagetool-language-modules/de/src/main/resources/org/languagetool/resource/de/multitoken-suggest.txt
(1 hunks)languagetool-language-modules/de/src/main/resources/org/languagetool/rules/de/grammar.xml
(1 hunks)languagetool-language-modules/de/src/main/resources/org/languagetool/rules/de/replace.txt
(1 hunks)languagetool-language-modules/en/src/main/resources/org/languagetool/resource/en/hunspell/spelling.txt
(1 hunks)languagetool-language-modules/en/src/main/resources/org/languagetool/resource/en/multiwords.txt
(1 hunks)languagetool-language-modules/en/src/main/resources/org/languagetool/rules/en/diacritics.txt
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- languagetool-language-modules/de/src/main/resources/org/languagetool/resource/de/hunspell/ignore.txt
- languagetool-language-modules/de/src/main/resources/org/languagetool/resource/de/hunspell/spelling.txt
- languagetool-language-modules/de/src/main/resources/org/languagetool/resource/de/multitoken-ignore.txt
🔇 Additional comments (14)
languagetool-language-modules/en/src/main/resources/org/languagetool/resource/en/hunspell/spelling.txt (1)
Line range hint 1-544
: LGTM! The additions enhance the spell checker's vocabulary.
The new entries:
- Follow the existing file structure
- Include comprehensive word forms (singular/plural, different tenses)
- Cover diverse fields (scientific, technical, cultural)
- Maintain alphabetical ordering
languagetool-language-modules/de/src/main/resources/org/languagetool/rules/de/replace.txt (3)
770-772
: LGTM: Proper noun and French term additions are correct.
The new entries correctly handle diacritical marks and follow proper formatting conventions.
Line range hint 773-870
: LGTM: German spelling reform updates are comprehensive and well-documented.
The changes correctly implement the 2024 German spelling reform requirements:
- Standardization of food terms (e.g., Spaghetti, Thunfisch)
- Proper handling of French loanwords with diacritical marks
- Updated spelling of common nouns
- Consistent pluralization of English loanwords ending in -y to -ys
Each entry includes helpful explanatory messages that clarify the reasoning behind the changes.
Line range hint 871-882
: LGTM: Compound word corrections are accurate.
The entries correctly handle:
- Separation of incorrectly joined adjective-noun combinations
- Proper capitalization rules
- Complete case variations for inflected forms
languagetool-language-modules/en/src/main/resources/org/languagetool/rules/en/diacritics.txt (2)
Line range hint 1-1368
: LGTM! Well-structured resource file.
The file follows a consistent and clear format with proper documentation.
Line range hint 1369-1427
: Verify consistency across language modules.
Since these diacritical corrections might be relevant for multiple languages, ensure that corresponding updates are made in related language modules where applicable.
languagetool-language-modules/de/src/main/resources/org/languagetool/resource/de/multitoken-suggest.txt (3)
3114-3117
: LGTM! The entries are properly formatted and consistent.
The new entries follow the established pattern with correct suffix indicators and maintain consistency with existing entries.
Line range hint 3118-3128
: LGTM! Entries are well-formatted and properly organized.
The new entries maintain consistency with:
- Correct suffix indicators
- Proper alphabetical ordering
- Appropriate mix of proper nouns and terms
Line range hint 3129-3130
: LGTM! Scientific name and proper name are correctly formatted.
The entries maintain proper formatting:
- Scientific name follows binomial nomenclature without suffix
- Proper name includes appropriate suffix indicator
languagetool-language-modules/en/src/main/resources/org/languagetool/resource/en/multiwords.txt (2)
Line range hint 1-4
: LGTM! Clear and comprehensive file header documentation.
The header comments effectively explain:
- Purpose of the file (English multiwords for disambiguation)
- Usage criteria (single POS terms only)
- Relationship with spelling.txt
- Case sensitivity handling
Line range hint 1-6021
: Verify consistency of POS tags for similar terms.
The file contains many cultural and religious terms. Let's verify consistent POS tagging across similar terms.
✅ Verification successful
POS tags are consistent for religious/cultural greetings and expressions
The verification shows that all similar religious/cultural greeting terms (salaam/assalamu/as-salamu alaikum and mazel/mazal tov) are consistently tagged as interjections (UH), which is appropriate for these expressions of greeting and blessing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check POS tag consistency for similar terms
# Test: Search for religious/cultural terms and their POS tags
rg "^(mazal|mazel|shalom|amen|hallelujah|alleluia|salaam|assalamu|as-salamu).+\s(UH|NNP|NN)" multiwords.txt
Length of output: 164
Script:
#!/bin/bash
# Description: Check POS tag consistency for similar terms
# Test: Search for religious/cultural terms and their POS tags from the repository root
rg "^(mazal|mazel|shalom|amen|hallelujah|alleluia|salaam|assalamu|as-salamu).+\s(UH|NNP|NN)" -g "multiwords.txt"
Length of output: 677
languagetool-core/src/main/resources/org/languagetool/resource/spelling_global.txt (3)
26717-26717
: Verify diaeresis usage in "Chloë".
The spelling "Chloë" correctly uses a diaeresis. This is consistent with the standard spelling of the actress's name.
26713-26714
: Consider consolidating name variants.
The file contains two variants of the same name: "Gideon Saar" and "Gideon Sa'ar". Consider documenting which is the preferred spelling or if both variants should be maintained.
Let's check for any existing documentation or patterns:
#!/bin/bash
# Description: Look for documentation about handling name variants
# Expected: Guidelines or similar patterns in other names
rg -l "name.*variant" --type md
26712-26718
: Investigate inconsistent line numbering.
Some entries have line numbers (26712-26718) while others don't. This inconsistency might indicate a structural issue in the file.
Let's check the file structure:
<rule> | ||
<pattern> | ||
<token regexp="yes">Dina[0-7]-(Format|Grö(ß|ss)e|Ma(ß|ss)e|Papier|Blatt|Druck|Poster|Rahmen|Plakat|Kalender|Werbe|Block|Bild|Brief|Zettel|Bogen).*</token> | ||
</pattern> | ||
<message>Diese Papiergröße wird in der Regel mit Bindestrichen geschrieben.</message> | ||
<suggestion><match no="1" regexp_match="(?i)Dina" regexp_replace="DIN-A" /></suggestion> | ||
<example correction="DIN-A4-Blatt">Ich schreibe auf ein <marker>DINA4-Blatt</marker>.</example> | ||
</rule> |
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.
🛠️ Refactor suggestion
Optimize pattern and enhance test coverage.
- Consider extracting the repeated noun list
(Format|Grö(ß|ss)e|...)
into a shared variable or entity to improve maintainability, as it's used in multiple rules. - Add more examples covering:
- Different suffixes from the pattern
- Various number combinations (A3, A5, etc.)
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation