NotePlan / plugins

The main NotePlan repository, which contains the source code for all NotePlan plugins. If a release entry has been created, it will be available for installation from within NotePlan application.
MIT License
167 stars 58 forks source link

Mechanism for extending Templates' {{tags}}? #11

Closed jgclark closed 2 years ago

jgclark commented 3 years ago

I've now got round to update my earlier '/dayStart' command (part of jgclark.DailyJournal plugin) in the light of the newer @nmn .Template plugin. In particular my aim was to use the template-configuration and {{tag}} mechanisms to integrate the weather lookup.

The first part was fine, though I am raising a PR to add a simpler version fo the addTemplate function, that can be used in a greater range of cases.

It's the second part I want to discuss here. Integrating my getWeatherSummary function was very easy once I worked out where I needed to integrate it the Template code (which wasn't commented): adding a simple if-clause to the processTag function. Which is fine at the moment. But if the number of plugin authors and plugins grow, this could become a bit of a bottle-neck on releases. Each new command that can be used in a template will need integrating at this point, which seems to be rather tightly coupling lots of different plugins to this one plugin. And will mean a new release each time for both the new plugin providing a tag, and for nmn.Templates.

Perhaps this is fine, and it effectively becomes a community 'np.Templates' over time. But I thought I'd ask now whether there might be a way to reduce the coupling and make it possible to define tags to be defined without changing processTags each time. cc @EduardMe

nmn commented 3 years ago

I’ve been thinking about this issue as well. As there is more and more shared parts to different plugins, each plug-in is now including duplicate code for the same stuff. I have two conflicting thoughts about this:

  1. If we can use roll up to share the code during development, the fact that the actual code is duplicated shouldn’t be much of a problem.
  2. If Noteplan itself could provide a way for one plug-in to run another plug-in, this problem wouldn’t exist.

For now, I think we van leverage Rollup to remove the development obstacles. We can continue to build plugins separately. Then we can just import certain functions from the Templates plug-in directly and we don’t have to do any additional development work.

jgclark commented 3 years ago

On your point 2 first, it seems to me that NP already allows us to run functions from other plugins, which we decide to do at development-time (your point 1). I can't see plugins usefully being able to discover other plugins at run-time and at that point decide to call them.

I want to check on your point 1 that when you say "share code" you're recommending I point to your code rather than take a copy of your code? I'm instinctively against same-code duplication where it can be avoided. The API will change and grow, and could affect our existing shared code: the imminent projectNoteByTitle change is a case in point. And perhaps in time Eduard will add Templates as a first-order object? That would change a lot of this code, then potentially spread across a number of different plugins from different authors that have copied in this code.

So if we're talking about pointing to your code, which is what I'm about to show you in the PR, we still come back to my original issue with processTags. It doesn't help us to copy that code to different plugins, as that would mean a Template couldn't use all available tags available to that user. So we need to find a way to best manage the coupling between other plugins and nmn.Templates.

The only additional thought I've had is that we don't actually need to declare a new release of .Templates every time we add a new tag handler. The updated code will get compiled in (via the helpful rollup), and will gradually spread as other authors update their plugins. None of this requires us to declare a new .Template version visible to users.

At this point I'm happy to continue as we are, if you're happy for others to be adding to nmn.Templates/src/*.js? In the upcoming PR I've added some comments and rename things to help other developers understand where to work, and for maintainers to check future additions.

(The PR is delayed while I check a detail with you (in discord) about parsing configuration.)

EduardMe commented 3 years ago

Thanks for this discussion. I'm not able to follow 100%. But it sounds like we discussed something similar before where we have something like global helper functions available to all plugins? I would rather opt for having copies of those helper functions to avoid that an update to them would mean the plugins using it would also need to be updated. Instead, the other plugins could be updated individually as needed by the original author. Sorry, if you discussed something different!

On Tue, Jun 8, 2021 at 5:48 AM Jonathan Clark @.***> wrote:

On your point 2 first, it seems to me that NP already allows us to run functions from other plugins, which we decide to do at development-time (your point 1). I can't see plugins usefully being able to discover other plugins at run-time and at that point decide to call them.

I want to check on your point 1 that when you say "share code" you're recommending I point to your code rather than take a copy of your code? I'm instinctively against same-code duplication where it can be avoided. The API will change and grow, and could affect our existing shared code: the imminent projectNoteByTitle change is a case in point. And perhaps in time Eduard will add Templates as a first-order object? That would change a lot of this code, then potentially spread across a number of different plugins from different authors that have copied in this code.

So if we're talking about pointing to your code, which is what I'm about to show you in the PR, we still come back to my original issue with processTags. It doesn't help us to copy that code to different plugins, as that would mean a Template couldn't use all available tags available to that user. So we need to find a way to best manage the coupling between other plugins and nmn.Templates.

The only additional thought I've had is that we don't actually need to declare a new release of .Templates every time we add a new tag handler. The updated code will get compiled in (via the helpful rollup), and will gradually spread as other authors update their plugins. None of this requires us to declare a new .Template version visible to users.

At this point I'm happy to continue as we are, if you're happy for others to be adding to nmn.Templates/src/*.js? In the upcoming PR I've already separated the key function into a separate file to make it easier to understand and check future additions.

(The PR is delayed while I check a detail with you (in discord) about parsing configuration.)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NotePlan/plugins/issues/11#issuecomment-856659050, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZCAYWBJBGEK5MDYDTGIUDTRXYQNANCNFSM46JBGX2Q .

nmn commented 3 years ago

if you're happy for others to be adding to nmn.Templates/src/*.js Yes I'm happy with this. Although I do think we should use Pull Requests for risky changes.

jgclark commented 3 years ago

I'm finally raising a PR that's designed to make it a bit easier for other newer plugin authors to know how the Template and function / tag extension mechanisms work. And how to extend them. I don't think I've changed functionality, though I have restructured some things.

nmn commented 3 years ago

I've been giving this some thought, and I believe there might be a way for users to extend {{tags}} with their own functions. This would need to rely on a code-fenced javascript bits.

Any practical implementation would involve validating the code though and that would increase the Templates plugin bundle by quite a lot.

Still, removing TOML and YAML will mostly equalize that, so we can consider this approach.

jgclark commented 3 years ago

Copying in some relevant comments from Discord yesterday, now I have remembered we have this open issue.

Many months ago when we first producing plugins, and @nmn was talking about the handy embedded {{functionCalls()}} that he went on to create, I suggested we look at Obsidian's Templater system. My suggestion was to try to borrow it's syntax, as we would know it had been field-tested for quite a while. We didn't do that ... but out of interest I read a piece by Stephen Millard automation-with-templater-for-obsidian which gave some more advanced examples of what he uses. Here's one case:

I now use Templater to create a new note, and a link to it, all in one expansion: [[<% (await tp.file.create_new(tp.file.find_tfile("Meeting - 1-2-1 Me (Template)"), tp.date.now("YYYY-MM-DD") + " 1-2-1 with ZY", false)).basename %>]]

I realised with a bit of a shock that this is actually a JavaScript line that gets embedded. That is certainly a different approach, and would give some additional flexibility.

@dwertheimer replied:

Yes, I was a fan of Obsidian Templater also. In reality, they have created a templating language, where we have mostly implemented (dynamic) text expansion. I continue to think that mirroring Obsidian's Templater in this regard is a big positive for many reasons. It's not like nmn.templates is so far along there's no switching horses.

I commented:

I wonder if approaching it this way would reduce the coupling into the Template plugin? Instead of its interpolation.js being the dispatcher for all function calls, NP itself would be doing that job. Each relevant plugin would provide a known extension point in its plugin.json file instead.

Now I see that @nmn's most recent comment is probably also saying a similar thing.

jgclark commented 3 years ago

This should definitely be a topic of conversation in our first plugin devs conversation, @EduardMe.

jgclark commented 3 years ago

FWIW, I think I've stumbled across what @nmn was maybe inspired by when he started the syntax we now use: jinja2 templating. It's a Python thing, so perhaps they're all more tolerant of {{strange punctuation}} than I am. Anyways, it's clearly a pretty mature and well-thought through templating system. I wonder if anyone has developed a JS version?

mikeerickson commented 3 years ago

@jgclark There are a few different templating libraries for JavaScript that have very similar syntax to Jinja2.

Nunjucks has been around for quite sometime and is pretty mature

Eta is a relatively new templating library, and is the defacto standard used by Obsidian Templater plugins (which we have discussed in the past)

dwertheimer commented 3 years ago

https://github.com/janl/mustache.js

On Thu, Sep 16 2021 at 4:28 AM, Jonathan Clark @.***> wrote:

FWIW, I think I've stumbled across what @nmn https://github.com/nmn was maybe inspired by when he started the syntax we now use: jinja2 templating https://jinja2docs.readthedocs.io/en/stable/templates.html. It's a Python thing, so perhaps they're all more tolerant of {{strange punctuation}} than I am. Anyways, it's clearly a pretty mature and well-thought through templating system. I wonder if anyone has developed a JS version?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NotePlan/plugins/issues/11#issuecomment-920819197, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACEI6VEPXSHX4BD6SSGI5S3UCHIHDANCNFSM46JBGX2Q .

jgclark commented 3 years ago

Aha, I sense a bigger picture emerging out of the mist. (Well, at least I had unseasonal mist here this morning.) And, yes, I meant to go back and look at the mustache reference, once I realised it wasn't a joke!

Maybe we should try to schedule another meet up with @EduardMe to work out if we should be aligning ourselves with one of these. (And perhaps do a plugin sprint retrospective, eh?!)

mikeerickson commented 3 years ago

I would concur we need to get @EduardMe involved for greater discussion. I have created a draft document and can share to the appropriate channel

jgclark commented 3 years ago

I would concur we need to get @EduardMe involved for greater discussion. I have created a draft document and can share to the appropriate channel

Excellent ... please share here as I think it's an extension of this still-open issue, though probably wider in scope. Please link to it in the Discord dev channel as well (I think you could do an @everyone post), so people know to come and look if interested.

mikeerickson commented 3 years ago

Please refer to the following discussion regarding NotePlan Plugin Templating

NotePlan Templating Plugin Discussion

EduardMe commented 3 years ago

We can have another meetup after the v3.1 release. Right now I‘m under self-imposed pressure to get this out. I‘m feeling a bit bad not having covered more plugin ground from my list.

The versioning / backup system took much longer than I anticipated (like so often). I have solved the main problems (performance of purging old versions) and now I‘m basically testing and fixing crash issues special to running a SQLite database.

Bad news is that the swift developer I hired to help me with features canceled. Looking for a new one.

EduardMe commented 3 years ago

But for the feature „Meeting Notes“ I would like to jump right away into templating and maybe even build this feature right away through a plugin

jgclark commented 3 years ago

Hire: that's a bummer. Hopefully you'll be able to quickly find a replacement.

Meeting notes: without access to any new UI elements I'm not clear what new Meeting Notes functionality we can give beyond the existing 'Meeting Note template'? But I look forward to being surprised by your ingenuity.

dwertheimer commented 3 years ago

We have too many places we are discussing things. I can’t remember where I wrote it. But my suggestion is this: Base case: /meetingNote 1)Opens up a chooser to select any event happening today, the default being something happening now on your calendar. 2) asks u what template u wanna use 3) feeds that info thru templates and populates the event info in the title and the note with templates

We can skip the second step for a variant command that has a template pre-selected in config

And we can create a version that skips all input if there’s only one thing on your calendar happening now (or in X mins) and u have a config setting for the template to use.

Ultimately, u should be able to right click in the calendar and skip the first step

I think there are Canny notes with more ideas also on this topic

On Sat, Sep 18 2021 at 10:28 AM, Jonathan Clark @.***> wrote:

Hire: that's a bummer. Hopefully you'll be able to quickly find a replacement.

Meeting notes: without access to any new UI elements I'm not clear what new Meeting Notes functionality we can give beyond the existing 'Meeting Note template'? But I look forward to being surprised by your ingenuity.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NotePlan/plugins/issues/11#issuecomment-922343540, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACEI6VCL5FYACX6ELM6KRKDUCTD4NANCNFSM46JBGX2Q .

EduardMe commented 3 years ago

Yes it‘s not much more than that. I would also show a little note icon on the calendar event and let users add it through right-click like you suggested.

jgclark commented 2 years ago

I'm hoping that Templating will help us significantly here. So I'm making this part of a new Milestone for it.

jgclark commented 2 years ago

In the dev call yesterday @mikeerickson confirmed that the Templating engine can be de-coupled from the Plugins that are providing functions that can be run in Templates. He has created a registration mechanism:

export async function templatingPluginModule(): Promise<void> {
  try {
    // create TemplatingEngine instance, it will be used to register plugins
    const templateInstance = new TemplatingEngine()

    // register templating plugin
    await templateInstance.register('bible', BiblePlugin)

    // TemplatingEngine does not have access to NotePlan API, thus we need to load template and pass to `.render` below
    const templateData: string = await NPTemplating.getTemplate('Test (Plugin Module)')

    // render template
    const result = await templateInstance.render(templateData)

    // write template rendered result
    Editor.insertTextAtCursor(result)
  } catch (error) {
    logError(error)
  }
}

He has also created a small plugin cd.FormattedDateTime which demonstrates how to implementing custom code into templates.

jgclark commented 2 years ago

@mikeerickson it has been dawning on me today that we still have tight coupling between (at least) np.Templating and jgclark.EventHelpers. I realised when I was banging my head against the table trying to work out why /insertDaysEvents was behaving differently from the same thing when used via a template 'tag'.*

Then @dwertheimer confirmed that he couldn't succefully run my updated EventHelpers plugin in a template unless he recompiled Templating.

So, I think we're back to this ancient issue. I know from your comments above (https://github.com/NotePlan/plugins/issues/11#issuecomment-1049342364) that you have thought about a registration system. But what I don't know is whether this is in place for the existing set of the various template tags that were in place before Templating, or if we need to re-plumb them in using this other system.

*I'm not a fan of the word 'tags' to refer to the functions. Would it be reasonable to call the <% ... %> things 'tags', and the functions they can contain as 'Templating functions'?

EduardMe commented 2 years ago

That's what we discussed yesterday with the solution of providing an API call to invoke commands of other plugins? And if the plugin can't find the command in the list, it can notify the user?

  1. const plugins = DataStore.installedPlugins()
  2. Search matching command in plugins
  3. A) command found: invoke with arguments, B) command not found, print or prompt user (print might be even better as it's less disruptive, but still parse the rest of the template, so we are not left with just an error message).

On Sat, Apr 23, 2022 at 6:42 PM Jonathan Clark @.***> wrote:

@mikeerickson https://github.com/mikeerickson it has been dawning on me today that we still have tight coupling between (at least) np.Templating and jgclark.EventHelpers. I realised when I was banging my head against the table trying to work out why /insertDaysEvents was behaving differently from the same thing used as a template 'tag'.*

Then @dwertheimer https://github.com/dwertheimer confirmed that he couldn't succefully run my updated code in a template unless he recompiled Templating.

So, I think we're back to this ancient issue. I know from your comments above (#11 (comment) https://github.com/NotePlan/plugins/issues/11#issuecomment-1049342364) that you have thought about a registration system. But what I don't know is whether this is in place for the existing set of the various template tags that were in place before Templating, or if we need to re-plumb them in using this other system.

*I'm not a fan of 'tags' to refer to the functions. Would it be reasonable to call the <% ... %> things 'tags', and the functions they can contain as 'Templating functions'?

— Reply to this email directly, view it on GitHub https://github.com/NotePlan/plugins/issues/11#issuecomment-1107666284, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZCAYWL3V5D47NKIBWVFDLVGSDEJANCNFSM46JBGX2Q . You are receiving this because you were mentioned.Message ID: @.***>

dwertheimer commented 2 years ago

I agree — print the error message in-line but process the rest of the template

On Wed, Apr 27, 2022 at 8:32 AM, Eduard Metzger @.***> wrote:

That's what we discussed yesterday with the solution of providing an API call to invoke commands of other plugins? And if the plugin can't find the command in the list, it can notify the user?

  1. const plugins = DataStore.installedPlugins()
  2. Search matching command in plugins
  3. A) command found: invoke with arguments, B) command not found, print or prompt user (print might be even better as it's less disruptive, but still parse the rest of the template, so we are not left with just an error message).

On Sat, Apr 23, 2022 at 6:42 PM Jonathan Clark @.***> wrote:

@mikeerickson https://github.com/mikeerickson it has been dawning on me today that we still have tight coupling between (at least) np.Templating and jgclark.EventHelpers. I realised when I was banging my head against the table trying to work out why /insertDaysEvents was behaving differently from the same thing used as a template 'tag'.*

Then @dwertheimer https://github.com/dwertheimer confirmed that he couldn't succefully run my updated code in a template unless he recompiled Templating.

So, I think we're back to this ancient issue. I know from your comments above (#11 (comment) https://github.com/NotePlan/plugins/issues/11#issuecomment-1049342364) that you have thought about a registration system. But what I don't know is whether this is in place for the existing set of the various template tags that were in place before Templating, or if we need to re-plumb them in using this other system.

*I'm not a fan of 'tags' to refer to the functions. Would it be reasonable to call the <% ... %> things 'tags', and the functions they can contain as 'Templating functions'?

— Reply to this email directly, view it on GitHub https://github.com/NotePlan/plugins/issues/11#issuecomment-1107666284, or unsubscribe https://github.com/notifications/unsubscribe-auth/ AAZCAYWL3V5D47NKIBWVFDLVGSDEJANCNFSM46JBGX2Q . You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/NotePlan/plugins/issues/11#issuecomment-1111146988, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACEI6VGVC2AJTTLVZTKCWQ3VHFMYJANCNFSM46JBGX2Q . You are receiving this because you were mentioned.Message ID: @.***>

dwertheimer commented 2 years ago

I wonder @mikeerickson if you can update the error mechanism in Templating to do this by default. Rather than stopping execution entirely, process the parts of the template that can be processed and print out the error in-line. Feels like a softer landing for users, and makes it even more obvious where the error exists. If they are in a hurry, they can delete those output lines and keep moving.

On Wed, Apr 27, 2022 at 9:09 AM, David Wertheimer @.***> wrote:

I agree — print the error message in-line but process the rest of the template

On Wed, Apr 27, 2022 at 8:32 AM, Eduard Metzger @.***> wrote:

That's what we discussed yesterday with the solution of providing an API call to invoke commands of other plugins? And if the plugin can't find the command in the list, it can notify the user?

  1. const plugins = DataStore.installedPlugins()
  2. Search matching command in plugins
  3. A) command found: invoke with arguments, B) command not found, print or prompt user (print might be even better as it's less disruptive, but still parse the rest of the template, so we are not left with just an error message).

On Sat, Apr 23, 2022 at 6:42 PM Jonathan Clark @.***> wrote:

@mikeerickson https://github.com/mikeerickson it has been dawning on me today that we still have tight coupling between (at least) np.Templating and jgclark.EventHelpers. I realised when I was banging my head against the table trying to work out why /insertDaysEvents was behaving differently from the same thing used as a template 'tag'.*

Then @dwertheimer https://github.com/dwertheimer confirmed that he couldn't succefully run my updated code in a template unless he recompiled Templating.

So, I think we're back to this ancient issue. I know from your comments above (#11 (comment) https://github.com/NotePlan/plugins/issues/11#issuecomment-1049342364) that you have thought about a registration system. But what I don't know is whether this is in place for the existing set of the various template tags that were in place before Templating, or if we need to re-plumb them in using this other system.

*I'm not a fan of 'tags' to refer to the functions. Would it be reasonable to call the <% ... %> things 'tags', and the functions they can contain as 'Templating functions'?

— Reply to this email directly, view it on GitHub https://github.com/NotePlan/plugins/issues/11#issuecomment-1107666284, or unsubscribe https://github.com/notifications/unsubscribe-auth/ AAZCAYWL3V5D47NKIBWVFDLVGSDEJANCNFSM46JBGX2Q . You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/NotePlan/plugins/issues/11#issuecomment-1111146988, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACEI6VGVC2AJTTLVZTKCWQ3VHFMYJANCNFSM46JBGX2Q . You are receiving this because you were mentioned.Message ID: @.***>

jgclark commented 2 years ago

Last night I think we agreed 2 things needed:

  1. Registration mechanism for plugin authors. This needs to cope with more functions than are exposed as '/commands'. Answer: use the 'hidden' tag on them in the plugin.json files, to hide from commandBar list.

  2. Templating to be able to look up registered functions. @EduardMe commented that he already had:

    listPlugins(_ showLoading: Bool) → Promise
    installPlugin(_ pluginObject: PluginObject, _ showLoading: Bool) → Promise
    installedPlugins() → [PluginObject]
  3. Templating to invoke commands after looking up what's registered. This needs a new invokeCommand(commandObject, [arguments]) → Promise command from @EduardMe.

jgclark commented 2 years ago

Am I right in thinking that this will mean the minAppVersion for plugins that use Templating will need to be 3.5.2?

I have committed mod to EventHelpers' plugin.json to add the listDaysEvents() and listMatchingDaysEvents() functions as hidden commands. Hopefully that will mean @mikeerickson and @EduardMe can test the new decoupling components.

EduardMe commented 2 years ago

Yes, if you want to use the new functions it has to be in 3.5.2, once ready.