Skip to content

Some new features and improvements #4

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
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

Mx-Iris
Copy link
Contributor

@Mx-Iris Mx-Iris commented Feb 16, 2025

This PR change is very large and is mainly used for RuntimeViewer application, which you can absolutely choose not to merge or partially merge.

  • Change ClassDump related fields to ClassDumpRuntime to avoid confusion with the ClassDump project.
  • Added comments for generating instance variable offsets.
  • Some interoperability optimizations with Swift.
  • Expanding structure member variables in the ObjC instance variables area (WIP).

This library is the core of RuntimeViewer and I will continue to add new features to it, here is what I think could be improved.

  • Extract the same C structures into a separate file (Currently direct expansion of structure members is very distracting to read)
  • Import the appropriate header files to make it work "out-of-the-box".

Finally, thank you very much for creating this library!

@leptos-null
Copy link
Owner

Thank you for publishing these changes and making a PR!
I'm very glad this framework was helpful to you, and you integrated it into your app- I'm interested to take a look at that project.

I've not started reviewing this PR yet. Based on the size, I may pull some of these changes into the repo with smaller commits. I'll discuss here before doing that. I'll definitely set the git author on those commits to you, if I do that.

Thanks again!

@Mx-Iris
Copy link
Contributor Author

Mx-Iris commented Feb 21, 2025

@leptos-null I have completed the implementation of expandIvarRecordTypeMembers option and can now expand nested structures or unions

@leptos-null
Copy link
Owner

Thank you!

I'm thinking about breaking the changes up into these commits:

  • Add addIvarOffsetComments to CDGenerationOptions
  • Add expandIvarRecordTypeMembers to CDGenerationOptions
  • Conform CDGenerationOptions to NSCopying
  • Conform CDSemanticString to NSSecureCoding

Changes in this PR that I'm considering to not merge:

  • Renaming the project from ClassDump to ClassDumpRuntime
    • I agree with the new name (it's a more unique name, and matches the name of this repository)
    • I want to check if this will break any existing projects as it's a rather large change
    • It looks like your renamed this very carefully throughout the project, including in the Makefile - thank you
  • NS_REFINED_FOR_SWIFT for CDGenerationOptions
    • I think this is pretty cool
    • I want to learn more about the Swift side of this being in a different target
      • I know that it has to be in a different target since Swift and Obj-C cannot be in the same Swift package
      • It seems odd to me that a project could import ClassDumpRuntime and not import ClassDumpRuntimeSwift. Would CDGenerationOptions be usable in this case?

Other changes:

  • Remove expand / indentLevel from CDRecordType
    • Move this logic into a new semanticStringForVariableName:indentLevel: method
      • I'm not sure exactly yet if expand also needs to be passed in
    • I do not think it is correct to put this information on the "type" itself, as "expand" does not directly relate to the type itself. I like the simplicity of your solution, for a somewhat complicated problem

(to be clear, I'm still thinking about some of these, just sharing them; I'll make all these changes, if I decide to, when I commit to this repo)

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.

2 participants