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

Markdown: allow the use of an ImageCache with MarkdownComponent #114

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DeDiamondPro
Copy link
Contributor

This pull request allows the use of an ImageCache with MarkdownComponent, this change should be backwards compatible.

Copy link
Contributor

@Johni0702 Johni0702 left a comment

Choose a reason for hiding this comment

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

this change should be backwards compatible.

It's not, that's why we have the explicit API file because otherwise it might not be obvious:
The constructor with the signature (Ljava/lang/String;Lgg/essential/elementa/markdown/MarkdownConfig;FLgg/essential/elementa/font/FontProvider;ILkotlin/jvm/internal/DefaultConstructorMarker;)V is gone now.
That is the constructor which Kotlin calls when you make use of default arguments (the final I is a bit mask telling the constructor which arguments were passed (so it can differentiate between not-passed and passed-null) and the final DefaultConstructorMarker is used to differentiate the constructor from an identical one with a real final I argument (otherwise such a constructor would conflict with the Kotlin-generated one).
To add an argument to a constructor which has optional arguments, you need to take the exact constructor, make it a secondary constructor so it's still there, and then create a new primary one that has the extra argument.

@DeDiamondPro DeDiamondPro force-pushed the markdown-image-cache branch 2 times, most recently from 611f612 to 07d1a3f Compare May 8, 2023 15:53
@DeDiamondPro
Copy link
Contributor Author

this change should be backwards compatible.

It's not, that's why we have the explicit API file because otherwise it might not be obvious: The constructor with the signature (Ljava/lang/String;Lgg/essential/elementa/markdown/MarkdownConfig;FLgg/essential/elementa/font/FontProvider;ILkotlin/jvm/internal/DefaultConstructorMarker;)V is gone now. That is the constructor which Kotlin calls when you make use of default arguments (the final I is a bit mask telling the constructor which arguments were passed (so it can differentiate between not-passed and passed-null) and the final DefaultConstructorMarker is used to differentiate the constructor from an identical one with a real final I argument (otherwise such a constructor would conflict with the Kotlin-generated one). To add an argument to a constructor which has optional arguments, you need to take the exact constructor, make it a secondary constructor so it's still there, and then create a new primary one that has the extra argument.

Thank you for the explanation, I wasn't aware this is how kotlin handles constructors. I think I have fixed it now.

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