Repository navigation
Conversation
Feature-based natural classes with no name are user-defined, since auto-generated natural classes are always assigned a non-empty name.
* Hide auto-generated feature-based NCs from displaying in NCs list.
* Create a RecordFilter subclass ("ExcludeAutoGeneratedNaturalClassFilter")
to filter out auto-gen NCs, and use it to filter the NC list in
areaConfiguration.xml.
* Hide auto-generated feature-based NCs from the phonological rule NC
choosers, so they can't be assigned to other APRs or phonological rules.
* Update AffixRuleFormulaControl.SetMappingNaturalClass and the
NaturalClass case of RuleFormulaControl.m_insertionControl_Insert
to show only NCs that are not auto-generated.
Cover ExcludeAutoGeneratedNaturalClassFilter: it rejects auto-generated classes, passes named, unnamed and segment-based classes, and still works when it receives its cache only through the Cache setter, as a filter restored from persisted settings does. Move the name matching out of IsFeatureBasedNCUserDefined into an internal IsAutoGeneratedName(ncName, format) helper, so it can be tested against format strings other than the English one. The behavior of IsFeatureBasedNCUserDefined is unchanged. Add tests that a null class is not auto-generated, that a format starting with the rule name recognizes generated names without matching ordinary ones, and that a format with no rule name placeholder recognizes nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment hygiene (advisory)No comment-style violations in the lines this branch adds since |
NUnit Tests 1 files ± 0 1 suites ±0 11m 23s ⏱️ - 1m 50s Results for commit cf36e63. ± Comparison against base commit 30e4078. This pull request removes 24 and adds 95 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1198 +/- ##
==========================================
+ Coverage 39.82% 39.88% +0.06%
==========================================
Files 1544 1543 -1
Lines 355187 355384 +197
Branches 41277 41325 +48
==========================================
+ Hits 141451 141752 +301
+ Misses 184386 184274 -112
- Partials 29350 29358 +8
🚀 New features to boost your workflow:
|
|
Unaddressed bugs mentioned by Devin:
This is a known possible but rare edge-case issue that can't currently be handled. After we make a model change to add a flag to NCs to track whether they were generated and do away with the naming-based detection method, this issue will go away.
This is intentional and is actually fixing what was a bug in the code. An unnamed feature class is user defined, not auto-generated, so its feature-editing command should not be visible. |
If we use a default hidden filter to hide auto-generated NCs, there are a couple of code paths that will circumvent the filter and cause autogen NCs to re-display in the NCs list. Instead, we remove them at the source--using a dynamic loader for the NC record list. Update tests to match new behavior. For clarity, rename test helper method from AddRuleNamedFeatureNaturalClass to AddAutoGeneratedFeatureNaturalClass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| { | ||
| protected override IEnumerable<int> GetObjectSet() | ||
| { | ||
| return ExcludeAutoGenerated(base.GetObjectSet(), m_cache); |
There was a problem hiding this comment.
Once a rule stops using one of its auto-generated classes, I can't find a way for the user to delete it now. Nothing deletes the class when the context is removed from the rule, when the rule itself is deleted, or when Set Mapping Features clears an affix mapping back to a copy (AffixRuleFormulaControl.SetMappingFeatures removes the mapping and leaves its class in NaturalClassesOS). Until now the user could find these leftovers in Grammar > Natural Classes and delete them; this list now hides them. They still show in the environment natural-class chooser, so the user will see "Created automatically for rule …" entries there that they can't remove.
Should a class be deleted when its rule stops using it? Or should the list still show the leftovers?
| /// True for feature-based natural classes generated automatically for a rule. | ||
| /// Phoneme-based classes are never auto-generated. | ||
| /// </summary> | ||
| public static bool IsAutoGeneratedNaturalClass(IPhNaturalClass nc) |
There was a problem hiding this comment.
This decides from the name alone, so an auto-generated class the user has taken over is hidden too. Two cases already have tests:
- the user gave it an abbreviation (
RuleNamedClass_AbbreviatedC_DrawsFeature), so environments can use it as[C]; - the user added text in front of the generated name (
ClassNamedForARuleBehindAPrefix_IsNotUserDefined).
In both cases the class now drops out of the Natural Classes list and the rule choosers, while environments and other rules may still use it. The user can no longer find it to edit it. Is that intended?
Fix IsFeatureBasedNCUserDefined logic
instead of auto-generated. Auto-generated feature-based
natural classes are always assigned a non-empty name.
Exclude auto-generated feature-based NCs from the NCs record list.
which excludes auto-generated NCs, and use it to dynamically
load the NC list in areaConfiguration.xml.
Hide auto-generated feature-based NCs from the phonological rule NC
choosers, so they can't be assigned to other APRs or phonological rules.
NaturalClass case of RuleFormulaControl.m_insertionControl_Insert
to show only NCs that are not auto-generated.
Note: auto-generated feature-based NCs still appear in environments.
After we have made a model change to add a flag to NCs to identify
auto-generated vs user-defined, we will be able to hide auto-generated
NCs in environment choosers as well.
This change is