nothingislost / obsidian-hover-editor

Transform the Page Preview hover into a working editor instance
MIT License
543 stars 21 forks source link

[FR] Trigger an event after patching `onLinkHover` #259

Closed RyotaUshio closed 8 months ago

RyotaUshio commented 8 months ago

Hi, thank you very much for this awesome plugin and a lot of inspiration.

As a plugin dev, sometimes I want to monkey-patch the onLinkHover method. However, Hover Editor completely replaces the original method with its own version on the workspace layout ready, and thus, any patchers that I apply to onLinkHover disappears if my plugin gets loaded before Hover Editor.

To bypass this, currently I'm using a dirty workaround:

But clearly, it's not a very ideal way. So it would be great if Hover Editor could trigger some event when patchLinkHover is done. For example:

patchLinkHover() {
    ...

    this.app.workspace.trigger("obsidian-hover-editor:on-link-hover-patched")
}

or

import { Events } from "obsidian";

export default class HoverEditorPlugin extends Plugin {
    events = new Events();

    patchLinkHover() {
        ...

        this.events.trigger("on-link-hover-patched")
    }
}

Thank you!

pjeby commented 8 months ago

It seems that not just an event, but also a flag would be needed since if your plugin is loading after HE then the patch is already there. Something to consider though. I'd like it better if I could just make monkey-around support priorities, such that HE could say "patch me in below any existing patches". Then normal patching would just work. I'd suggest you patch the instance rather than the prototype, but the way monkey-around currently works it still wouldn't handle it correctly.

Though, thinking about it some more, if I make monkey-around support prototype delegation, then you'd be able to just patch the pagePreview instance, while HE would patch the prototype, and then it'd all be fine. And I wouldn't even need to make a new release of HE, you'd just use the newer monkey-around in your plugin and patch the instance instead of the prototype.

I'm not sure which of these strategies I'll actually do; have to give it some thought and perhaps some experimentation. The instance vs. prototype thing has kind of been a potential issue with how monkey-around works for some time now, though, so it might be nice to get it fixed.

RyotaUshio commented 8 months ago

Thank you for the prompt response!

not just an event, but also a flag would be needed

You're right, thank you for pointing it out.

Personally, I think supporting priorities might be better since it will be also able to handle priorities within instance patchers or within prototype patchers.

Your work on monkey-around is great, it has helped a lot of plugins including some of mine. Thank you very much a lot for your effort.

pjeby commented 8 months ago

Yeah, the reason I switched from priorities to prototype hopping is that it doesn't require every plugin to be upgraded with the new monkey-around. Right now, monkey-around doesn't wrap functions in a way that would allow prioritization to work unless all installed plugins used the new monkey-around. And the instance vs. prototype priority (and subclass/superclass) priority is already an issue with monkey-around now, in that if you patch an instance or subclass and then later patch the class or one of its base classes, then you will have a problem right now. Since fixing that issue also makes it possible to patch the instance here, it seems like the way to go.

RyotaUshio commented 8 months ago

Thank you, that's an interesting story to hear. Fixing this will doubtlessly bring a lot of benefits to plugin developers so I will check the code base to see if I can help.

pjeby commented 8 months ago

Ok, I have published monkey-around 3.0.0 with the prototype hopping. So try upgrading your plugin to use that, and then just patch the instance of the page preview plugin rather than its constructor's .prototype. That should make it so that HE's patch can be done before or after yours, without any ill effect. You should be able to just turn HE on and off and have your plugin still work at all times.

If not, let me know and we'll try to figure it out further.

RyotaUshio commented 8 months ago

Hi, monkey-patch v3.0.0 combined with instance patching works very well! Thank you very much for the quick update. I really appreciate it.

Now that no event will be needed to avoid the conflict, I'm closing this issue.