ozntel / oz-image-in-editor-obsidian

This Obsidian plugin to view Images, Transclusions, iFrames and PDF Files within the Editor without a necessity to switch to Preview.
339 stars 13 forks source link

[BUG] Images inside a link are broken #11

Closed pjeby closed 3 years ago

pjeby commented 3 years ago

If you have a line like this:

* [![](https://example.com/some.jpg) Link text](https://example.com)

The result is a corrupted image, whose src is something like https://example.com/some.jpg) Link text](https://example.com. The problem appears to be the use of .* in some of your regexes, in places where they should instead be [^)]* (i.e. match any character that's not a closing parenthesis.)

ozntel commented 3 years ago

Hi @pjeby, thank you for your feedback. To be honest, it is the first time I see such a usage pattern combining both images and links.🙂

Before I release the new version, would you be able to check if the correction of link regex helps to solve this issue for you:

https://github.com/ozntel/oz-image-in-editor-obsidian/blob/665f93d03f8da6c5775c51025d1a9d570afafc0e/src/utils.ts#L26

I also updated the main.js in the repo in case you would like to try it.

As you mentioned, the links were detected as all text starting from the opening parenthesis until the last closing parenthesis.

pjeby commented 3 years ago

It fixed it for that example, yes.

ozntel commented 3 years ago

Thanks for confirming. I released the new version with this update.

Since I have you here, I would like to ask you something if you don't mind 🙂

I released a new plugin for cleaning the unused images in the vault. Right now plugin has only a command to run. However, I am planning to implement an option for users to automate the process of clearing the images every X minutes or each time they load the app.

During the load, if I don't include a timeout, the plugin deletes all the images since resolved links are not set yet. I added 3 seconds to make sure that they are set but do you maybe know any other way, which would be safer to implement here to ensure that resolved links are set before plugin runs during the load:

https://github.com/ozntel/oz-clear-unused-images-obsidian/blob/b8fae6e6ded6efca5de593842fc3ac9d5147b786/src/main.ts#L29

Thanks in advance!

pjeby commented 3 years ago

You can use app.metadataCache.onCleanCache(callback) to run the callback as soon as the cache is clean (i.e., not busy parsing or resolving). The callback then read a valid/consistent state regarding links from the metadata cache, as long as it is purely synchronous -- i.e. not an async function. If you will be performing any async operations from the callback you must either save the promises and then do them later, or else build the list of files to delete in the callback and then do the deletion afterward. This is the only way to ensure that the cache is clean for the duration of your processing.

(Note that you should be doing this for all invocations of your cleaning function, not just the first one.)

It would also probably be a good idea to not delete images that might be new (e.g. have a recent modify or creation time), so that if you've just added an image but the file hasn't saved and the interval triggers it doesn't delete what you just added. Similarly, having an option to control what directory or directories will be cleansed would be a good idea as well. (Because for example, people probably don't want to delete the images generated by the Excalidraw plugin, or images that they've manually added for whatever reason without using.)

ozntel commented 3 years ago

Thanks a lot for your input. Do you maybe know why onCleanCache doesn't exist in the public API and if it might cause an error in mobile?

pjeby commented 3 years ago

It's not in the public API because the Obsidian creators tend to worry about changing documented things and so expose the minimum API possible, even if it means a dozen plugins have to reinvent the same wheels over and over.

As for mobile, I don't see why it would break. AFAICT there is only one Obsidian codebase for both desktop and mobile and there is nothing desktop-specific used by that function.