Skip to content

fix(editline): failing test with correct out-of-bounds behavior#228

Merged
Tieske merged 3 commits intolunarmodules:mainfrom
Gitkbc:fix-editline-subchar-test
Apr 3, 2026
Merged

fix(editline): failing test with correct out-of-bounds behavior#228
Tieske merged 3 commits intolunarmodules:mainfrom
Gitkbc:fix-editline-subchar-test

Conversation

@Gitkbc
Copy link
Copy Markdown
Contributor

@Gitkbc Gitkbc commented Mar 18, 2026

Replaces the pending test for sub_char out-of-bounds behavior.

Instead of returning an empty string, it now follows Lua’s string.sub behavior by resolving indices to the nearest valid position.

Updated the test accordingly and removed the pending block. All tests pass.

Copy link
Copy Markdown
Member

@Tieske Tieske left a comment

Choose a reason for hiding this comment

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

the test was written in a way that it validates that whatever EditLine does (the val variable), matches the standard Lua behaviour (the exp variable).

I think we should retain this behaviour. So the test is right, but the implementation is not in this case.

@Gitkbc Gitkbc force-pushed the fix-editline-subchar-test branch from cdd38d3 to 5cdd311 Compare April 2, 2026 14:18
@Gitkbc
Copy link
Copy Markdown
Contributor Author

Gitkbc commented Apr 2, 2026

Thanks for the review. You were completely right about retaining that test. I reverted the test back to the exp == val logic and removed the pending status. To make it pass, I updated the sub_char implementation so it properly clamps the indices and returns an empty string only when the start exceeds the end or the length. It now perfectly mimics Lua's native string.sub behavior.

Please let me know if you’d like any further refinements.

@Tieske Tieske changed the title fix(editline): replace pending test with correct out-of-bounds behavior fix(editline): failing test with correct out-of-bounds behavior Apr 3, 2026
@Tieske Tieske merged commit 926bc42 into lunarmodules:main Apr 3, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants