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

fix: unescaped quotes breaking zk json output #403

Closed
wants to merge 7 commits into from

Conversation

tjex
Copy link
Member

@tjex tjex commented Apr 7, 2024

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

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.
@mickael-menu
Copy link
Member

mickael-menu commented Apr 7, 2024

I don't think the tesh case was incorrect, the new output with this fix has the JSON string delimiters escaped, which is not valid JSON:

{\"filename\":\"without-title\",\"path\":\"without-title\",\"absPath\":\"/tmp/config-format-markdown-link.tesh-1114206750/blank/without-title\",\"relPath\":\"without-title\",\"title\":\"\",\"metadata\":{}}

I recommend adding a tests/issue-389.tesh to exhibit the problem and validate a fix.

@tjex
Copy link
Member Author

tjex commented Apr 8, 2024

Thanks for chiming in on this one 🙏

That's so weird about the tesh case, whereby the quotes in the json fields themselves become escaped.
Because if I install from this branch and run zk graph --format json with a note, another "test" note.md, the json fields themselves do not become escaped as you show in the snippet. Only the quotes within the json value fields:

{
  "filename": "another \"test\" note.md",
  "filenameStem": "another \"test\" note",
  "path": "another \"test\" note.md",
  "absPath": "/Users/tjex/.local/src/zk-org/workbench/test-vault/another \"test\" note.md",
  "title": "another \"test\" note",
  "link": "[[another \"test\" note]]",
  "lead": "",
  "body": "",
  "snippets": [],
  "rawContent": "# another \"test\" note\n\n\n",
  "wordCount": 4,
  "tags": [],
  "metadata": {},
  "created": "2024-04-01T03:11:05.217425491Z",
  "modified": "2024-04-01T03:11:05.217630323Z",
  "checksum": "a121f8d06da1bff78c5e5eb4816d0417ab2ec22317bad43baedb51a20c60df68"
}

Also discussed here with some more detail.

In any case, I'll keep looking into it.

Copy link

github-actions bot commented May 9, 2024

This pull request has been automatically marked as stale because it has not had recent activity.

@github-actions github-actions bot added the stale No recent activity label May 9, 2024
tjex added 2 commits May 10, 2024 10:09
It should pass, but it fails because we're not
sanitizing double quotes in note titles properly.
@tjex tjex added bug Something isn't working and removed stale No recent activity labels May 10, 2024
@tjex
Copy link
Member Author

tjex commented May 10, 2024

fixed in an atrocious way. But for the life of me I can't find where the json formatting functionality actually happens from notebook.NewNoteFormatter("{{json .}}") in graph.go, see here.

I'm go-to-definitioning in circles...

If anyone can chime in with where the formatting actually happens via the template, that would be ace. To my novice eyes, it's bordering on actual magic.

Other solution could be to unpack the object in line 70 of graph.go and to sanitize there?

If this cli filtering fix is however a better way to go, I would look then into kong and see how I can query in a more robust way than searching for strings in the os.Args[] slice 🥲🔫

@tjex tjex closed this by deleting the head repository May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zk graph --format json is not handling note titles with inner double quotes
2 participants