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 missing images in export #128

Closed
wants to merge 2 commits into from
Closed

fix missing images in export #128

wants to merge 2 commits into from

Conversation

dirkroorda
Copy link

See the comments in the code.

Test it before applying it, since I did not do any building-bundling whatsoever.

@evolve2k
Copy link

evolve2k commented Oct 20, 2022

Fix looks sensible to me.

@OliverBalfour could you look this over? It doesnt look like a very high risk change at all.

Image rendering images in the Obsidian Pandoc flow are affecting many people, this looks like it might solve a bunch of them.

People reporting (likley related) image exporting Issues:
#66
#81
#95
#118
#138

https://forum.obsidian.md/t/how-to-pass-image-directory-location-to-pandoc/33349
https://forum.obsidian.md/t/pandoc-does-not-show-embedded-images-in-docx/27209

@evolve2k
Copy link

evolve2k commented Oct 20, 2022

@OliverBalfour I can see you've worked on this a bit as discussed in your Future Directions post.

I reviwed your patch code, and this change is separate from what you've already done.

@dirkroorda dirkroorda thanks for this patch, could you clean up and remove all the comments except for the code change lines, that'll make it easier for someone to just hit MERGE. I think here is the best place for disclaimers and workflow discussions, as you've done. Would be good to get this in if it's such a small simple fix and as you said it's working for you.

@dirkroorda
Copy link
Author

@evolve2k Hi Richie, just did it.

@skillhacker-code
Copy link

Looking forward to this update

@kw90
Copy link

kw90 commented Nov 23, 2022

Is there something missing here or could we merge this? This would improve my workflow a lot! Thanks for your work! 🙂

@evolve2k
Copy link

@OliverBalfour? Can you help us all out.

Hoping it's a very easy merge now. It's a 2 line change.

@Interpause
Copy link

It seems this patch only works for markdown mode, not html mode, referring to the below lines:

obsidian-pandoc/main.ts

Lines 106 to 146 in 68ff9e3

case 'html': {
const { html, metadata } = await render(this, view, inputFile, format);
if (format === 'html') {
// Write to HTML file
await fs.promises.writeFile(outputFile, html);
new Notice('Successfully exported via Pandoc to ' + outputFile);
return;
} else {
// Spawn Pandoc
const metadataFile = temp.path();
const metadataString = YAML.stringify(metadata);
await fs.promises.writeFile(metadataFile, metadataString);
const result = await pandoc(
{
file: 'STDIN', contents: html, format: 'html', metadataFile,
pandoc: this.settings.pandoc, pdflatex: this.settings.pdflatex,
directory: path.dirname(inputFile),
},
{ file: outputFile, format },
this.settings.extraArguments.split('\n')
);
error = result.error;
command = result.command;
}
break;
}
case 'md': {
const result = await pandoc(
{
file: inputFile, format: 'markdown',
pandoc: this.settings.pandoc, pdflatex: this.settings.pdflatex,
directory: path.dirname(inputFile),
},
{ file: outputFile, format },
this.settings.extraArguments.split('\n')
);
error = result.error;
command = result.command;
break;
}

@EllriNidhogg
Copy link

So, the problem is pulling the path information from the file object when passing the .md file. It looks like the current code passes the raw HTML from stdin, which lacks path data. So a potential fix would be to generate a temporary HTML file in the directory of the input file, then remove it once the export is complete. It's kind of awkward but it solves the problem from the pandoc command line point of view?

@MavropaliasG
Copy link

Will this patch allow for images in PDF and odt too?

@dirkroorda dirkroorda closed this by deleting the head repository Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants