zk-org / zk

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

fix quotes in titles breaking graph json output #400

Closed tjex closed 6 months ago

tjex commented 6 months ago

resolves: #389

note titles with " marks in them, eg this is an "example" title, are breaking the output of zk graph --format json.

The culprit is the template for the json format. I can't exactly track down where that is set, but it would seem deeper from internal/core/notebook.go:336.

In any case, this fix manually sanitizes the quote marks as they're being returned to the Link field, for output to json. Link output within documents (i.e, inserting wiki/markdown links in documents is not broken as that is handled separately in internal/core/link_format.go).

The caveat is that this fix breaks an old tesh test that looks at json output.

However, it would seem that the old tesh test is not taking into account note titles with " marks.

Therefore, the failing test may be a sign of the test needing to be updated, rather than this fix being void?

In any case, this initial commit to the PR is effectively a draft. I'm looking for feedback about the above!

There is also [a discussion] in a previous issue which shows the development of the graph output, and also that quotes in the titles are breaking its output

tjex commented 6 months ago

@mickael-menu Would it be ok to get some high level input from you on this one? As you had commented on the quotes breaking json before. Also the usage of this template functionality has got me a bit stumped:

func (n *Notebook) NewNoteFormatter(templateString string) (NoteFormatter, error) {
    templates, err := n.templateLoaderFactory(n.Config.Note.Lang)
    if err != nil {
        return nil, err
    }
    template, err := templates.LoadTemplate(templateString)
    if err != nil {
        return nil, err
    }

    linkFormatter, err := NewLinkFormatter(n.Config.Format.Markdown, templates)
    if err != nil {
        return nil, err
    }

    return newNoteFormatter(n.Path, template, linkFormatter, n.osEnv(), n.fs)
}

So I'm not that confident that I'm looking at this fix in the right way, and as the fix is changing the functionality that is also accessed by zk list, I'm wary of creating future bugs down the way.

tjex commented 6 months ago

accidentally pushed here from gh cli instead of my own repo..