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

feat(#769): Add j$ Prefix For All Method Names #775

Merged
merged 4 commits into from
Oct 20, 2024

Conversation

volodya-lombrozo
Copy link
Member

@volodya-lombrozo volodya-lombrozo commented Oct 20, 2024

In this PR I added the j$ prefix for all method names. This prevents some problems in EO and PHI naming.
For example, the $φTerm java method will be converted to the j$φTerm.
During the back transformation jeo removes this prefix and saves pure φTerm to bytecode.

Related to: #769.


PR-Codex overview

This PR introduces the PrefixedName class to enhance method name handling by adding a prefix to method names in the DirectivesMethod and XmlMethod classes. It also includes a test to verify that the prefix is correctly applied.

Detailed summary

  • Updated DirectivesMethod to use PrefixedName for encoding method names.
  • Added a test in DirectivesMethodTest to confirm the prefixing of method names.
  • Updated XmlMethod to use PrefixedName for decoding method names.

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@volodya-lombrozo
Copy link
Member Author

@yegor256 Could you take a look this one as well, please?

Copy link
Member

@yegor256 yegor256 left a comment

Choose a reason for hiding this comment

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

@volodya-lombrozo
Copy link
Member Author

@rultor merge

@rultor
Copy link
Contributor

rultor commented Oct 20, 2024

@rultor merge

@volodya-lombrozo OK, I'll try to merge now. You can check the progress of the merge here.

@rultor rultor merged commit 7e41469 into objectionary:master Oct 20, 2024
11 checks passed
@rultor
Copy link
Contributor

rultor commented Oct 20, 2024

@rultor merge

@volodya-lombrozo Done! FYI, the full log is here (took me 9min).

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