phildenhoff / logseq-raindrop

A Raindrop plugin for Logseq
MIT License
62 stars 5 forks source link

Add support for color template var in highlight template #8

Closed luhmann closed 1 year ago

luhmann commented 2 years ago

Thank you for the plugin.

I noticed that the {color}-variable in the highlight template does not get replaced by the actual color value as described in the template description: https://github.com/phildenhoff/logseq-raindrop/blob/9fc490613bc5927f17a9a7a67ad00c3004988753/src/util/settings.ts#L23 Instead the string "{color}" is printed as is. I investigated the issue real quick and saw that there was no code that supports the functionality.

I created a quick PR in case you want to add support as specified in the description (after looking at the code, I felt that it actually might make more sense to add it as a block-property next to annotation-id). So if it was just the description that was erroneous feel free to close the PR.

phildenhoff commented 2 years ago

hmmm yeah I'm not 100% sure, it would probably make more sense to add it as a block property — if we add it to the block content, it just kinds of clutters it up?

I guess I don't really have a good use case for why you would want to see the colour yet. Do folks use it to organise thoughts? Different kinds of highlights? What is your preference?

luhmann commented 2 years ago

I use different colors to highlight different thoughts(eg. red = quotable statement that can be used on its own, blue = main question that gets examined in the article, green = further research this thought).

When I applied this fix, I indeed did not find the result to be that useful as you cannot copy and paste the text as is.

I also tried out adding it as a block property and confirmed that it can be used in logseq queries this way. This might be handy, but personally I did not use it much so far. Then again it is fairly unobtrusive, easy to implement and might provide value to someone with a different workflow.

I would be happy to create a different PR that implements the block property and removes the mention of {color} as a variable from the helptexts if that is what you want to do.

phildenhoff commented 1 year ago

Closing this as it sounds like you don't find the change very useful as-is, and I haven't found the need for it either. If that changes, or you'd like this in, let me know and we can re-open this.