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

Fix/program client custom types #117

Merged
merged 4 commits into from
Feb 15, 2024
Merged

Conversation

lukacan
Copy link
Collaborator

@lukacan lukacan commented Feb 5, 2024

update parse_to_idl_program so that it can obtain the full path for custom types. The implemented function will warn the user if the path is private and cannot be imported.

This is out of scope, but I found it useful to update the examples.

Added some custom types into the cargo test expected/expanded code so that the functionality can be tested.

@lukacan lukacan requested a review from Ikrk February 5, 2024 17:24
@lukacan lukacan force-pushed the fix/program_client-custom-types branch 2 times, most recently from 1efa8e4 to 00e1e89 Compare February 14, 2024 13:16
@lukacan
Copy link
Collaborator Author

lukacan commented Feb 14, 2024

@Ikrk rebased #115 , ready to review

Copy link
Collaborator

@Ikrk Ikrk left a comment

Choose a reason for hiding this comment

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

Lgtm, I would suggest to only optimize the parsing as suggested in the other comment...

}

fn find_item_path(code: &str, target_item_name: &str) -> Option<String> {
let syn_file = syn::parse_file(code).ok()?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is not necessary to call the expensive parse_file function all over again every time the find_item_path is called. It would be better to pass it as a reference to the parsed file that is parsed at the beginning of the parse_to_idl_program function and therefore do the parsing only once...

@lukacan lukacan requested a review from Ikrk February 14, 2024 23:20
Copy link
Collaborator

@Ikrk Ikrk left a comment

Choose a reason for hiding this comment

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

I have corrected a problem when a program has a hyphen "-" in its name. This resulted into "unexpected token" error. Converting the name to snake case fixed the issue.

Also, it would be nice to get rid of the unnecessary cloning in the future, but this could be done in a separate PR.

@Ikrk Ikrk force-pushed the fix/program_client-custom-types branch from a615c4b to 19a56da Compare February 15, 2024 12:59
@Ikrk Ikrk merged commit e69029a into develop Feb 15, 2024
7 checks passed
@Ikrk Ikrk deleted the fix/program_client-custom-types branch February 15, 2024 13:07
Ikrk added a commit that referenced this pull request Feb 15, 2024
* obtain full path for custom types (Struct/Enum), provide good UX if types are private and cannot be used within program_client

* ✅ add cargo test for program_client

* 🔥 removed duplicate code

* 🐛 Fixed malformed program name.

---------

Co-authored-by: lukacan <andrej@DESKTOP-UME2160.localdomain>
Co-authored-by: Ikrk <ikrk@centrum.cz>
lukacan added a commit that referenced this pull request May 20, 2024
* obtain full path for custom types (Struct/Enum), provide good UX if types are private and cannot be used within program_client

* ✅ add cargo test for program_client

* 🔥 removed duplicate code

* 🐛 Fixed malformed program name.

---------

Co-authored-by: lukacan <andrej@DESKTOP-UME2160.localdomain>
Co-authored-by: Ikrk <ikrk@centrum.cz>
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.

None yet

2 participants