Skip to content
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

Lexicon suffixes (Fixes #2551) #2553

Merged
merged 2 commits into from
Jul 26, 2019
Merged

Conversation

Dunbaratu
Copy link
Member

Fixes #2551 Fixes #2288

Making this work required a few things:

1 - Overriding GetSuffix and SetSuffix so that if a Lexicon
fails to Get or Set suffix on the normal suffixes, it falls
back to trying it on the keys.

2 - Making it so that GetSuffix and SetSuffix are sometimes
allowed to fail without throwing an exception, since expecting
them to sometimes fail is now part of Lexicon's normal execution
path it doesn't seem right to "communicate" that fact to
Lexicon with an exception it has to catch.

3 - Also overriding HASSUFFIX and SUFFIXNAMES so they reflect
the fact that some lex keys effectively become "suffixes" now.

Making this work required a few things:

1 - overriding GetSuffix and SetSuffix so that if a Lexicon
fails to Get or Set suffix on the normal suffixes, it falls
back to trying it on the keys.

2 - Making it so that GetSuffix and SetSuffix are sometimes
allowed to fail without throwing an exception, since expecting
them to sometimes fail is now part of Lexicon's normal execution
path it doesn't seem right to "communicate" that fact to
Lexicon with an exception it has to catch.

3 - Also overriding HASSUFFIX and SUFFIXNAMES so they reflect
the fact that some lex keys effectively become "suffixes" now.
The test for if a string is a valid identifier was allowing
partial matches to count, so for example the string:

  "StartsLikeIdentifierButHas *garbage*!(# afterward"

was getting called a valid identifier by the Lexicon suffix
system.
@Dunbaratu Dunbaratu added the Pending Documentation Author is warning Reviewer not to merge this PR yet. User doc changes aren't in the PR yet. label Jul 18, 2019
@Dunbaratu
Copy link
Member Author

I Just noticed that this PR is missing Sphinx doc changes to describe the new feature. It should not be merged without that.

@Dunbaratu Dunbaratu changed the title Lexicon suffixes (Fixes #2551) [Peinding documentation] Lexicon suffixes (Fixes #2551) Jul 18, 2019
@Dunbaratu Dunbaratu changed the title [Peinding documentation] Lexicon suffixes (Fixes #2551) [Pending documentation] Lexicon suffixes (Fixes #2551) Jul 18, 2019
@Dunbaratu Dunbaratu merged commit 35fa997 into KSP-KOS:develop Jul 26, 2019
@Dunbaratu Dunbaratu removed the Pending Documentation Author is warning Reviewer not to merge this PR yet. User doc changes aren't in the PR yet. label Jul 28, 2019
@Dunbaratu Dunbaratu changed the title [Pending documentation] Lexicon suffixes (Fixes #2551) exicon suffixes (Fixes #2551) Jul 28, 2019
@Dunbaratu Dunbaratu changed the title exicon suffixes (Fixes #2551) Lexicon suffixes (Fixes #2551) Jul 28, 2019
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.

Lexicons could use suffix syntax by overriding GetSuffix(). Do not make list#x deprecated it works for OOP
1 participant