-
Notifications
You must be signed in to change notification settings - Fork 145
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
Conversation
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.
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 |
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. {
"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. |
This pull request has been automatically marked as stale because it has not had recent activity. |
This reverts commit be46e73.
It should pass, but it fails because we're not sanitizing double quotes in note titles properly.
Fix is however not idiomatic and is substandard, therefore, consider as functional but a wip.
fixed in an atrocious way. But for the life of me I can't find where the json formatting functionality actually happens from 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 |
resolves: #389
note titles with
"
marks in them, egthis is an "example" title
, are breaking the output ofzk 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 ininternal/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