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 #274 - Overwrite svg file if input file is output #275

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

Conversation

gistrec
Copy link

@gistrec gistrec commented Apr 6, 2021

This PR solves #274

@gistrec gistrec changed the title Fix #274 - Allow to overwrite svg file Fix #274 - Overwrite svg file if input file is output Apr 6, 2021
@gistrec gistrec force-pushed the same-input-and-output-file branch from e90931b to c19130f Compare April 6, 2021 18:33
@Ede123
Copy link
Member

Ede123 commented May 1, 2021

Thanks for this!

Some thoughts

  • I'm still thinking about border cases that might destroy the original file content when something goes wrong - we should make absolutely sure this can not happen. My first thought was to write to a temporary file first, and move/rename later, but that might have implications of it's own and might mean unnecessary file IO.
  • I find the approach of opening one file in r+ mode and still treating it as two separate files (input/output) a bit convoluted and see the danger of introducing bugs at one point if we're not careful. That said I admit it is an elegant solution in principle, but maybe we can still come up with something better, that makes it clear it's only one file or actually handles reading and writing separately?
  • I'd prefer the option name --in-place in line with the original issue Support in-place scrubbing #129

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.

None yet

2 participants