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

Allow to use paths with extensions in image exporter #277

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

tomaspavlic
Copy link

Allow to use paths with extensions in the image exporter. This seems more natural when working with paths. To be backwards compatible add extension when it does not exists otherwise check if provided extension is correct if so use it - otherwise throw an exception to warn about misuse of export function.

Comment on lines +22 to +26
let ensureFileExtension (ext: string) (path: string) =
match Path.GetExtension(path) with
| "" -> sprintf "%s%s" path ext
| e when String.Compare(e, ext, StringComparison.OrdinalIgnoreCase) = 0 -> path
| _ as e -> failwithf "Provided path contains wrong file extension. Expected '%s' got '%s'." ext e
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I'd just write wherever the user wants to. I think it's not smart to dictate the user what extension should be. Why can't we just write a jpg to a txt file? It would make it transparent


return rendered |> getBytesFromBase64String |> fun base64 -> File.WriteAllBytes($"{path}.jpg", base64)
return rendered |> getBytesFromBase64String |> fun base64 -> File.WriteAllBytes(path, base64)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I 💯 agree with this change. Adding silent .jpg is imo not a good idea.

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.

2 participants