Fix attachment interoperability between win and *nix

[saaguero]

Proposal for fixing #2139.

The main idea is to:

• Allow either / or \ as part of the :storage paths in the markdown content. That way we preserve compatibility with already created documents.

• Adjust RegExp in the code to handle this and then sanitize to use OS dependent path separator. This allow us to show images in win and *nix platforms without touching the markdown content (very useful if you version your notes in 3rd party storage like Dropbox)

• As a consequence of these changes, no deletion of attachments will happen when opening the note between win and *nix.

• It would be very nice to open a PR for making the "deletion of attachments" feature configurable.

[ehhc]

Hi there,
first of all: thanks for fixing this issue. I'm sorry i haden't had time to do it myself. I hope changing my code wasn't that painful (i hope i produced good code.. :D )

Considering your two todos: First: yes as far is i know the migrateAttachment method is used and is working (at least there is nobody complaining anymore)..
Second: I think using it at a different location so that it can use the same getAttachmentsInContent (resp. in MarkdownContent..) method would be possible and good (less redundant code..).
One problem with it: It's hard to test that it is still working correctly afterwards.. And we would have to test it a lot so that we don't cause the problems again, like i did when i refactored that code initially. Steps to test:

• go back to 0.11.4
• insert a image via drag & drop into a note
• check that the images was copied to :storage/images
• upgrade to your branch
• have a look at the note
• check:
  1. The image is visible in the note content
  2. The image was copied to :storage/attachments

Can you have a look if you can refactor that as well? And test that it still works? @saaguero ??

Otherwise: your PR looks good to me. Thanks for your work :)

[saaguero]

Hey @ehhc, the code was easy to follow.

I'm going to check about the TODO and see if I can refactor it a little bit, adding unit test and doing the regression test as you proposed.

Will check it later today.

[saaguero]

I've slightly changed the migrateAttachment function to take the markdown content instead of the rendered HTML. I've also added a basic unit test.

Finally, I've performed the regression test proposed by @ehhc in both Fedora and Windows 10; seems to be working fine but any additional testing around this is appreciated.

[ehhc]

your changes look good to me :) 👍
(but i have to admit, i don't have the time for the regression test at the moment :( )

[Rokt33r]

I confirmed it works on macOS.