Allow multiple embedded file types.

[jheddings]

Uses the generalized block type for uploading files.

Resolves #29

[Cobertos]

Do all the if statements still apply to this use case?

if re.search(r"(?<!file)://", imgRelSrc, re.I):
    return #Don't upload images that are external urls
if imagePathFunc: #Transform by imagePathFunc insteadif provided
    imgSrc = imagePathFunc(imgRelSrc, mdFilePath)
else:
    imgSrc = relativePathForMarkdownUrl(imgRelSrc, mdFilePath)
    if not imgSrc:
        print(f"ERROR: Local image '{imgRelSrc}' not found to upload. Skipping...")
        return
print(f"Uploading file '{imgSrc}'")
newBlock.upload_file(str(imgSrc))

I guess it works for your use case because Day One was putting video and PDF attachments in Markdown image syntax? Otherwise I'd think you'd run into issues with relativePathForMarkdownUrl when trying to load your files.

The more I look at this, the more I don't understand why imagePathFunc didn't just default to relativePathForMarkdownUrl, but maybe with a function partial on mdFilePath,,, A chore for the future I think

I would add a comment in the code that it's expecting an ImageBlock but also supports other blocks, just to clarify why it's the super classes. but otherwise LGTM

[jheddings]

Do all the if statements still apply to this use case?

Yes, they are still valid, since it is still possible to reference an external file. That would still be rendered as a standard image block.

I guess it works for your use case because Day One was putting video and PDF attachments in Markdown image syntax? Otherwise I'd think you'd run into issues with relativePathForMarkdownUrl when trying to load your files.

Correct... It uses the image syntax as a generic "embedded" media type.

The more I look at this, the more I don't understand why imagePathFunc didn't just default to relativePathForMarkdownUrl, but maybe with a function partial on mdFilePath,,, A chore for the future I think

In my case, I was changing the block type before calling upload to ensure that Notion used the right block. uploadBlock ended up skipping my blocks since they were of type VideoBlock of PDFBlock.

I would add a comment in the code that it's expecting an ImageBlock but also supports other blocks, just to clarify why it's the super classes. but otherwise LGTM

Good call. Done.

[Cobertos]

Okeee, sounds good. I'll get this merged tonight. Thank you

On Fri, Feb 5, 2021, 7:43 PM Jason Heddings notifications@github.com wrote:

Do all the if statements still apply to this use case?

Yes, they are still valid, since it is still possible to reference an external file. That would still be rendered as a standard image block.

I guess it works for your use case because Day One was putting video and PDF attachments in Markdown image syntax? Otherwise I'd think you'd run into issues with relativePathForMarkdownUrl when trying to load your files.

Correct... It uses the image syntax as a generic "embedded" media type.

The more I look at this, the more I don't understand why imagePathFunc didn't just default to relativePathForMarkdownUrl, but maybe with a function partial on mdFilePath,,, A chore for the future I think

In my case, I was changing the block type before calling upload to ensure that Notion used the right block. uploadBlock ended up skipping my blocks since they were of type VideoBlock of PDFBlock.

I would add a comment in the code that it's expecting an ImageBlock but also supports other blocks, just to clarify why it's the super classes. but otherwise LGTM

Good call. Done.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/Cobertos/md2notion/pull/30#issuecomment-774362511, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTSGCGI5OVK7TMSPGVUINLS5SGBNANCNFSM4XFNASQA .

[Cobertos]

Thanks for the PR!