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

Give plugin authors the chance to extend a palette #6624

Merged
merged 6 commits into from
Apr 16, 2022

Conversation

BurningTreeC
Copy link
Contributor

This PR changes the colour macro so, that it transcludes a tiddler with the name of the current palette but with the suffix /extension on the second position

This gives plugin authors the chance to extend palettes

@BurningTreeC
Copy link
Contributor Author

bad idea @Jermolene ?

@Jermolene
Copy link
Member

Hi @BurningTreeC I think this approach only allows one plugin to provide extensions for any particular palette. We should always be trying to minimise the chance of conflicts between plugins.

An approach that allows plugins to provide entries for particular palettes without conflicting with one another might be to support config tiddlers of the form $:/config/DefaultColourPaletteMappings/<palette>/<name>:

<$transclude tiddler={{$:/palette}} index="$name$">
<$transclude tiddler="$:/palettes/Vanilla" index="$name$">
<$transclude tiddler={{{ [[$:/config/DefaultColourPaletteMappings/]addsuffix<__palette__>addsuffix[/]addsuffix<__name__>] }}}/>
<$transclude tiddler="$:/config/DefaultColourMappings/$name$"/>
</$transclude>
</$transclude>
</$transclude>

The only conflicts with that design is if two plugins try to set the same entry for the same palette, but that's an intrinsic conflict that we can't resolve.

@BurningTreeC
Copy link
Contributor Author

Hi @Jermolene , I've changed it as you said
I'll add some docs

@Jermolene
Copy link
Member

Thanks @BurningTreeC I meant to add that we should also take this opportunity to replace the text substitution usage of this macro, please.

@BurningTreeC
Copy link
Contributor Author

Thanks @Jermolene , I've updated everything accordingly

Copy link
Member

@Jermolene Jermolene left a comment

Choose a reason for hiding this comment

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

Thanks @BurningTreeC just a minor point.

core/wiki/macros/CSS.tid Show resolved Hide resolved
@Jermolene
Copy link
Member

Thanks @BurningTreeC

@Jermolene Jermolene merged commit b3b3020 into TiddlyWiki:master Apr 16, 2022
@BurningTreeC BurningTreeC deleted the patch-50 branch April 16, 2022 15:50
Jermolene added a commit that referenced this pull request Apr 16, 2022
@Jermolene
Copy link
Member

@BurningTreeC I have reverted this commit because it completely breaks colour handling:

image

Did you test it before submission?

@BurningTreeC
Copy link
Contributor Author

Apologies @Jermolene - I tested it but there's one small typo

@BurningTreeC BurningTreeC restored the patch-50 branch April 16, 2022 16:56
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