Skip to content

Fix for LT-18767 Yellow crash foe XAmple-style redup pattern#313

Closed
AndyBlack wants to merge 1 commit intomasterfrom
LT18767
Closed

Fix for LT-18767 Yellow crash foe XAmple-style redup pattern#313
AndyBlack wants to merge 1 commit intomasterfrom
LT18767

Conversation

@AndyBlack
Copy link
Copy Markdown
Collaborator

@AndyBlack AndyBlack commented Jun 23, 2025

When using an XAmple-style reduplication pattern, if there was an indexed natural class in the pattern that was not also in the environment, then Hermit Crab threw a key not found exception. This by-passes that.

Note: I wanted to create a unit test case for this but did not know how to create the reduplication information in an Allomorph. So there is no unit test case for this.


This change is Reviewable

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.82%. Comparing base (90dcee6) to head (e8e486c).

Files with missing lines Patch % Lines
...ogy.HermitCrab/MorphologicalRules/CopyFromInput.cs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #313      +/-   ##
==========================================
- Coverage   70.83%   70.82%   -0.01%     
==========================================
  Files         390      390              
  Lines       32729    32731       +2     
  Branches     4609     4610       +1     
==========================================
+ Hits        23182    23183       +1     
- Misses       8483     8484       +1     
  Partials     1064     1064              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

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

It would be good if we had a unit test that covers this case.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @AndyBlack)

@AndyBlack
Copy link
Copy Markdown
Collaborator Author

Yes, I agree it would be good to have a unit test for this. As I said above, though, I don't know how to create an Allomorph with an XAmple-like reduplication pattern.
Can you help with that, please?

Copy link
Copy Markdown
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

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

Oops, I missed that. I will see what I can do.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @AndyBlack)

Copy link
Copy Markdown
Contributor

@ddaspit ddaspit left a comment

Choose a reason for hiding this comment

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

Now that I understand the issue a bit better, I feel like it is more appropriate to fix this issue in FieldWorks. From HC's perspective, this is an invalid case and shouldn't be allowed. Since this is specific to the reduplication pattern in FieldWorks, it would make sense to fix it in the HCLoader class in FieldWorks. The LoadReduplicationOutputActions can be updated to ignore output actions that use a name that doesn't exist on the LHS. You can get a list of the valid names from the return value of LoadReduplicationPatterns. Each returned pattern object has a Name property.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @AndyBlack)

@AndyBlack
Copy link
Copy Markdown
Collaborator Author

OK. Sounds good. I'll work on that in HCLoader.cs.
I'm closing this pull request.

@AndyBlack AndyBlack closed this Jun 24, 2025
@ddaspit ddaspit deleted the LT18767 branch September 5, 2025 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants