Skip to content

TREMLEntities.MapToEntity doesn't perform per description #84

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

Open
delphidabbler opened this issue Dec 23, 2022 · 3 comments
Open

TREMLEntities.MapToEntity doesn't perform per description #84

delphidabbler opened this issue Dec 23, 2022 · 3 comments
Assignees
Labels
bug Bug report completed Work has been completed on this issue and changes have been committed to `develop` branch..

Comments

@delphidabbler
Copy link
Owner

class function TREMLEntities.MapToEntity(const Ch: Char): string;

Description says this function returns the character itself if it is printable ASCII, but it returns the empty string in this case.

@delphidabbler delphidabbler added bug Bug report needs investigation Investigation required to confirm problem considering Issue under consideration. May be accepted or rejected labels Dec 23, 2022
@delphidabbler delphidabbler self-assigned this Dec 23, 2022
@delphidabbler
Copy link
Owner Author

Need to check how this method is used. If calling code depends on out of spec behaviour then update comments else correct the code.

@delphidabbler
Copy link
Owner Author

delphidabbler commented Apr 13, 2023

TREMLEntities.MapToEntity gets called from this method:

function TREMLWriter.TextToREMLText(const Text: string): string;

This code tests the method's return value against the empty string, so this is what it is expecting.

So, it looks like the method acts as expected and the comments are wrong.

I think the actual behaviour is reasonable because the code is supposed to be return an entity if one exists. Furthermore, the calling code has to wrap the entity between & and ; characters, so the empty string result is useful to help deciding if this should be done.

Maybe renaming to MapToEntityIfExists would clarify.

@delphidabbler delphidabbler moved this to Considering in CodeSnip May 23, 2023
@delphidabbler
Copy link
Owner Author

The comments for both TREMLEntities.MapToEntity and the private TREMLEntities.CharToMnemonicEntity methods were changed to reflect their actual functions.

@delphidabbler delphidabbler added accepted Accepted for implementation / fixing and removed considering Issue under consideration. May be accepted or rejected needs investigation Investigation required to confirm problem labels Apr 15, 2025
@delphidabbler delphidabbler moved this from Considering to In progress in CodeSnip Apr 15, 2025
delphidabbler added a commit that referenced this issue Apr 15, 2025
The comments for both TREMLEntities.MapToEntity and the private
TREMLEntities.CharToMnemonicEntity methods were changed to reflect their
actual functions.

Fixes #84
@delphidabbler delphidabbler added completed Work has been completed on this issue and changes have been committed to `develop` branch.. and removed accepted Accepted for implementation / fixing labels Apr 15, 2025
@delphidabbler delphidabbler moved this from In progress to Done - awaiting release in CodeSnip Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report completed Work has been completed on this issue and changes have been committed to `develop` branch..
Projects
Status: Done - awaiting release
Development

No branches or pull requests

1 participant