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

If a hook is used in more than one active script, it is executed only in one of them #663

Closed
MilanRusev opened this Issue Aug 9, 2017 · 10 comments

Comments

Projects
None yet
2 participants
@MilanRusev

MilanRusev commented Aug 9, 2017

Hi,

It seems if more than one script uses the same hook it is executed for only one of them. I came across it while developing the integration for markdown-it. Both https://github.com/qownnotes/scripts/tree/master/markdown-it and https://github.com/qownnotes/scripts/tree/master/preview-styling use noteToMarkdownHtmlHook() and when I enable both packages it was executed in only one of them.

Expected behaviour

noteToMarkdownHtmlHook() (and any other hook) should be executed for every script that is enabled.

Additionally it may be a good idea to introduce a dependency/priority mechanism between scripts to be able to specify the call order.

Actual behaviour

The hook is executed in only one of the enabled scripts.

Steps to reproduce

  • Enable each of preview-styling and markdown-it scripts separately and make different visual configurations to them. E.g.:
    • In preview-styling leave the default h2 {margin-left: 20px;}
    • In markdown-it enable line breaks
  • Enable both scripts - the effect of only one of them is applied (in my case it was markdown-it)

Although there is a custom stylesheet setting in markdown-it, this is not the cause - I saw the behaviour without it (and in fact that is the reason I added it as a workaround for now)

@pbek

This comment has been minimized.

Owner

pbek commented Aug 9, 2017

Yes, currently it's designed to stop after the first script-call was detected.
see:

if (!text.isEmpty()) {
return text;
}

Since the order in which scripts are executed can't be changed right now I'm not sure if that should be changed. I think all the hooks work that way currently...

@pbek pbek added the enhancement label Aug 9, 2017

@pbek pbek added this to the 17.08.3 milestone Aug 10, 2017

@pbek

This comment has been minimized.

Owner

pbek commented Aug 10, 2017

17.08.3

  • you can now reorder your scripts in the Script settings via drag and drop
    to adjust which scripts should be executed first
  • the scripting function noteToMarkdownHtmlHook can now be used in multiple
    scripts to modify the html of the preview
@pbek

This comment has been minimized.

Owner

pbek commented Aug 10, 2017

There now is a new release, could you please test it and report if it works for you?

@pbek

This comment has been minimized.

Owner

pbek commented Aug 10, 2017

@Maboroshy this is maybe also relevant to you ;)

@MilanRusev

This comment has been minimized.

MilanRusev commented Aug 11, 2017

Wow, thank you! I tested it with preview-styling and markdown-it (I added manually a script.log() in both scripts' hooks) and both get called. However the calling order isn't changed when I rearrange them in the script settings. I tried restarting the application and it's still the same.

I haven't done a clean install, just upgraded the app.

@pbek

This comment has been minimized.

Owner

pbek commented Aug 11, 2017

Thank you for reporting, I will take a look at that (I hope it's something that can be fixed)

@pbek

This comment has been minimized.

Owner

pbek commented Aug 13, 2017

I rewrote those parts now.

17.08.4

  • the order of the scripts in the Script settings now really adjusts which script is executed first

@pbek pbek modified the milestones: 7.08.4, 17.08.3, 17.08.4 Aug 13, 2017

@pbek

This comment has been minimized.

Owner

pbek commented Aug 13, 2017

There now is a new release, could you please test it and report if it works for you?

@MilanRusev

This comment has been minimized.

MilanRusev commented Aug 14, 2017

Now it works, thanks!

@pbek

This comment has been minimized.

Owner

pbek commented Aug 14, 2017

Great, thank you for testing!

@pbek pbek closed this Aug 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment