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

Added incrementOnFilenameConflict Option #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

PeteMichaud
Copy link

Added incrementOnFilenameConflict option to optionally avoid overwriting existing output files.

For example, if pandoc is about to output to "MyFile.docx" but that file already exists, then this option will increment the filename to "MyFile1.docx," or whatever the appropriate increment number is like "MyFile6436.docx".

@AB1908
Copy link

AB1908 commented Apr 17, 2022

IMO, it's better to stay consistent with pandoc's behavior and overwrite and instead pop up a modal asking the user if they want to overwrite. What do you think?

@PeteMichaud
Copy link
Author

That's certainly an option, but it wouldn't work for my use case.

I'm outputting a document into a locally mounted google drive, then (using a macro) following the pandoc command with a command that gives me the gdrive share link for that document, so if I want to get feedback on an obsidian document I hit Cmd+Shift+S and it automatically gets formatted, uploaded, and I get a share link to give out.

The problem I was solving with this is that the original behavior silently overwrote whatever file had been there before, which includes whatever comments readers had made on the google doc. So for example if I shared one version one day and tried to share another version on a different day, the new version would silently blow away the old one, along with whatever edits and comments existed. No bueno.

So for me it's not a matter of "Overwrite? Yes/No," it's a matter of "The share command should never destroy data."

A different solution I considered, that may still be useful for others, is something like a Save As modal. Right now the filename can only be exactly whatever the note filename is (modulo extension), but a popup offering Overwrite or Save As or Cancel would be a natural addition to the tool. This using the OS's Save As dialog via something like remote.dialog.saveSaveDialog().

I guess the full feature would have:

  • a separate command that gave you as Save As dialog no matter what, so users could optionally save the file with whatever name, regardless of currently existing files
  • the existing command now checking for the file's existence and offering the Overwrite/Save As/Cancel dialog
    • this should be an option that defaults to false, in favor of the existing behavior of silently overwriting, to avoid breaking people's workflow.

Still, for my usecase where I never want the tool to be able to overwrite anything, and expect to generate multiple versions over time, and I want it to "Just Work" without any additional user interaction, the increment is better than Save As.

@AB1908
Copy link

AB1908 commented Apr 17, 2022

Your patch is totally understandable given your use case but given that we want to migrate to this fork being the "official" one, I'm a bit hesitant to merge something like this (first time maintainer here). Perhaps use your own releases via BRAT for now? argenos is fairly busy but I'd probably want her opinion on this as well.

@argenos, if you wouldn't mind chiming in.

@chrisgrieser
Copy link
Member

I think there are three different options when the output file already exists, And there are arguments / use cases for all three:

  • overwrite without confirmation (pandoc's default behavior)
  • confirmation-to-override modal (what @AB1908 suggested)
  • increment instead of overwrite (@PeteMichaud's PR)

So I think having a setting where the user can select one of the three would make most sense?

@chrisgrieser
Copy link
Member

just so it doesn't get lost: here is a guide on how to use modals in Obsidian plugins

…re overwrite, and numerical increment to avoid overwrite
@PeteMichaud
Copy link
Author

Ok, just pushed a crack at the feature as discussed. I have only tested on desktop, but I don't anticipate any issues, the code is pretty straightforward.

@PeteMichaud
Copy link
Author

Ok, I may have uncovered an issue while helping Laura on the discord channel, but I can't reproduce it. I'd like either of you, @AB1908 or @chrisgrieser to try the following steps:

  1. Don't use my fork, just use the current community release of the pandoc plugin
  2. Remove any custom pandoc output path from your settings, just leave it blank.
  3. Try to generate any file via pandoc that is not from the vault root directory. It probably isn't important, but it happened to be the case that the generated file in Laura's case was a pdf, so maybe try that.
  4. Now use my fork and do step 2 again.

That's it. The correct behavior is for the new file to end up in the folder you generated it from, so like generating from Vault/MyFolder/MyFile.md should produce Vault/MyFolder/MyFile.pdf

The behavior that Laura found while using my fork is that MyFile.pdf ended up in the vault root folder.

I'd like to know whether this bug is new to my fork or if it exists in the current version. I'll fix it either way, but I want to address it separately if it's separate!

@AB1908
Copy link

AB1908 commented Jul 1, 2022

I couldn't generate a pdf file at all with your fork unfortunately. The correct behavior is found on the current fork.

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.

3 participants