allow '=' in values of pandoc key=value pairs - #814
Conversation
|
Thanks for the PR! We need to be super careful with stuff like this - not that pandoc_attr.ts is particularly careful, but I don't want to make situation worse. I think I'd want some tests around this behavior so we don't regress on it, but also understand the blast radius of the change. |
|
@cscheid and @kevinushey the current tests (which did pass on CI) do provide some decent coverage on this. It would be great to add some roundtripping snapshot tests (.e.g https://github.com/quarto-dev/quarto/pull/790/files) that more explicitly capture the new behaviour though! |
|
Thanks for adding tests @kevinushey!! Gonna take a look at this. |
| return lines.map(line => { | ||
| const parts = line.trim().split('='); | ||
| return [parts[0], (parts[1] || '').replace(/^"/, '').replace(/"$/, '')]; | ||
| const idx = line.indexOf('='); |
There was a problem hiding this comment.
Realized post-hoc that I should probably check if the line here actually has an =?
There was a problem hiding this comment.
yea I suppose so! like if (idx === -1) return [line, '']; else { bla }
There was a problem hiding this comment.
attr-equals.qmd is quite extensive! Nice. Perhaps it could also contain some lines of prose/markdown that contain equals symbols? Not sure if that could cause issues or not.
Also, could you add a description of the fix to https://github.com/quarto-dev/quarto/blob/main/apps/vscode/CHANGELOG.md under 1.125.0 please?
Looks great otherwise! Excited to have this fix! The test makes me pretty confident that this works well!
Please squash and merge once the PR is ready.
|
Thanks! I updated the change log, and added a couple more "weird" examples for the roundtrip test. I don't have permissions to merge here so I'll ask you to do so if everything looks good. |
e68a02e to
11e0a9f
Compare
3 participants
Closes #813.