sethyuan / logseq-plugin-kanban-board

Draggable, editable Kanban view.
MIT License
59 stars 2 forks source link

Tags are not split #16

Closed almereyda closed 10 months ago

almereyda commented 10 months ago

When delimiting tags:: with a comma (,), they are not displayed as separate entities.

grafik

grafik

This may be due to an uncommon character used in the split rule, and missing square brackets to indicate the characters as equally valid matches:

https://github.com/sethyuan/logseq-plugin-kanban-board/blob/748ca1a7a29c210c38cab8c97f892995bea8e98e/src/libs/utils.js#L21

The suggestion is to add the square brackets and a space character:

<    for (const t of tagsPropValue.split(/,,/)) {
>    for (const t of tagsPropValue.split(/[,, ]/)) {

I'm testing this with a local build, before PRing.

sethyuan commented 10 months ago

Yes you're right. Thanks for spotting this bug! I can fix this right now, or I can wait for your PR.

almereyda commented 10 months ago

@sethyuan There it is. I tried to keep it minimal, but needed to test locally first. Where I found the need for an additional .filter(), so this was a good step to validate and not commit blindly.

Will be happy not having to run from my custom fork.

sethyuan commented 10 months ago

It seems that there is an extra space character in the PR's regex, I removed it, FYI. A new version is released.

almereyda commented 10 months ago

Thank you for the quick reaction!

I fear it's needed, else the space is prepended to the next tag:

Before After
grafik grafik

grafik

Do you see the little shift?

sethyuan commented 10 months ago

Oh, I see, let me deal with it.

almereyda commented 10 months ago

@sethyuan Out of curiosity, may I ask why you are using the character as separator?

It seems unusual in European contexts, and VS Code warns about it, but I don't know if it is actually used somewhere in the rest of the world, why I am asking.

sethyuan commented 10 months ago

Sure, it is the Chinese full-width comma character. I added this so that Chinese users can keep typing comma without switching the IME first.

almereyda commented 10 months ago

Very good!