zk-org / zk

A plain text note-taking assistant
https://zk-org.github.io/zk/
GNU General Public License v3.0
1.54k stars 114 forks source link

`zk tag` doesn't shows numeric tags #420

Closed Rooyca closed 1 day ago

Rooyca commented 1 month ago

Check if applicable

Describe the bug

I have a file with this tag #2024 and when I use zk tag the tag is not displayed, if a change it for something like #2024/06 the tag appears.

If I use :2024: it works. If I use #2024# it doesn't.

How to reproduce?

  1. Create a note with one of this tags: #2024 #2024#
  2. Run zk tag
  3. It doesn't show the tags

zk configuration

[note]

template = "default.md"

[extra]

[format.markdown]

hashtags = true

colon-tags = true

multiword-tags = true

[tool]

[lsp]

[lsp.diagnostics]

dead-link = "error"

[lsp.completion]

[filter]

[alias]

Environment

zk 0.14.0
system: Linux 6.9.3-arch1-1 x86_64 GNU/Linux
tjex commented 3 weeks ago

#tag# is not a valid tag. So that wont work. It's only colon tags that are formatted like this, :tag:.

But yes, #2024 also does not show up, while #2024a does.

Placing in frontmatter also doesn't work:

---
tags: 2024
---

I'll have a look and see if this is a bug or perhaps by design.

Rooyca commented 3 weeks ago

tag# is not a valid tag

Is just a #multi-word tag#?

I'll have a look and see if this is a bug or perhaps by design.

Thanks.

tjex commented 3 weeks ago

True yes, #this is a multi-word tag#.

How confident are you in doing the initial investigation yourself btw? (just saw you ticked the box 😜)

Rooyca commented 3 weeks ago

I was doing that, I remember I came to a part where I didn't understand what was going on with the code and give up. I could try it again tho.

tjex commented 3 weeks ago

Have another crack and just write here with where you get up to :) If you like and find it fun / a learning exercise. If not, then not a stress!

Rooyca commented 3 weeks ago

I'll do it :smile:

Rooyca commented 3 weeks ago

Hey @tjex, I believe I found something at internal/adapter/markdown/extensions/tag.go.

It looks like the tags cannot be numbers. But only when using #.

func isValidHashTag(tag string) bool {
    for _, char := range tag {
        if !unicode.IsNumber(char) {
            return true
        }
    }
    return false
}

Is this a bug or a feature? Looks like is intended but I don't understand why.

tjex commented 3 weeks ago

Great find 👌 I'll have a look and see if I come up with anything.

tjex commented 2 weeks ago

I can't see any reason in the code why a hash tag cannot be a number. I also can't find any markdown spec or standards pointing towards a number not being a valid hash tag.

@mickael-menu , can you remember the reasoning behind this choice? isValidHashTag is only used once on line 162 along with a check for 0 character length.

    tag = strings.TrimSpace(tag)
    if len(tag) == 0 || !isValidHashTag(tag) {
        return nil
    }
tjex commented 1 day ago

@Rooyca While I can't see any specific reason why hashtags can't be numbers. It has been designed this way, and changing it could (naturally) have unintended consequences. Perhaps something odd goes on with sql parsing, or some other process.

What makes me particularly hesitant is that there have been specific tests for this made by Mickael:

https://github.com/zk-org/zk/blob/00a4361d20690d78e2f431261908cf550ec26215/internal/adapter/markdown/markdown_test.go#L185-L186

He most definitely knows what he's doing, so without him reporting back by now to reconsider an initial decision, I'd prefer to request that you add a letter in there somewhere to make it a valid tag, e.g, #Y2024, rather than me going about changing code and a respective test.

mickael-menu commented 1 day ago

I couldn't think of a good use case for a pure number tag, and treating it as tags can trigger a lot of false positive.

As @tjex mentioned, I'd recommend prefixing the number with a letter to qualify it.