obsidian-tasks icon indicating copy to clipboard operation
obsidian-tasks copied to clipboard

Add hooks to let other plugins extend the Tasks plugin.

Open alexthewilde opened this issue 3 years ago • 29 comments

Description

Add hooks to let other plugins extend the Tasks plugin.

Motivation and Context

I needed a way to extend the Tasks plugin with my own functionality (see discussion).

How has this been tested?

Screenshots (if appropriate)

Here's an example of how other plugins can make use of the new hooks to extend the Tasks plugin's functionality:

Use like image

Renders as image

Implemented in new plugin

export const extendTask = (listItem: HTMLLIElement, task: any, api: any) => {
  if (api.layoutOptions.showDoTodayButton) {
    addDoTodayButton(listItem, task, api);
  }
}

export const extendParser = (line: string, layoutOptions: any) => {
  if (line === 'show do today button') {
    layoutOptions.showDoTodayButton = true;
    return true;
  }
  
  return false;
}

function addDoTodayButton(listItem: HTMLLIElement, task: any, api: any) {
  const today = window.moment();
  const isToday = task.scheduledDate && task.scheduledDate.format('YYYY-MM-DD') === today.format('YYYY-MM-DD');
  const doTodayBtn = listItem.createEl('a', {
      text: isToday ? 'not today' : 'do today',
      cls: 'tasks-do-today',
  });

  doTodayBtn.onClickEvent((event: MouseEvent) => {
      const updatedTask = new api.Task({
          ...task,
          scheduledDate: isToday ? null : today,
      });

      api.replaceTaskWithTasks({
          originalTask: task,
          newTasks: [updatedTask],
      });
  });
}

Types of changes

Changes visible to users:

  • [ ] Bug fix (prefix: fix - non-breaking change which fixes an issue)
  • [x] New feature (prefix: feat - non-breaking change which adds functionality)
  • [ ] Breaking change (prefix: feat!! or fix!! - fix or feature that would cause existing functionality to not work as expected)
  • [ ] Documentation (prefix: docs - improvements to any documentation content)
  • [ ] Sample vault (prefix: vault - improvements to the Tasks-Demo sample vault)

Internal changes:

  • [ ] Refactor (prefix: refactor - non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)
  • [ ] Tests (prefix: test - additions and improvements to unit tests and the smoke tests)
  • [ ] Infrastructure (prefix: chore - examples include GitHub Actions, issue templates)

Checklist

  • [x] My code follows the code style of this project and passes yarn run lint.
  • [x] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [ ] My change has adequate Unit Test coverage.

Terms

alexthewilde avatar Oct 22 '22 18:10 alexthewilde

Wow. It looks amazing. Huge thanks for working on this. It would open up some really exciting avenues with the plugin.

That's a lot to get my head around, in not very much code... It might also give ideas of how to easily add new button functionality in to the existing Tasks plugin.

How to test it

I notice you haven't filled in the testing section. How are you imagining I might test it? I don't suppose you could share a link to your test plugin please, so that it could be tested more quickly and easily?

GitHub Actions

I see that the 'verify' GitHub Action failed. Looks like there are some ESLint issues????

Implications for future changes to the Tasks code

What impact would it have on future refactoring of the Tasks code? If this were published, for example, would it prevent changes to the interface of the Task class? Asking because there is a lot of public data that I would want to protect at some point... I would hate to break a load of people's customisations - but would also prefer not to have our hands tied for future refactorings...

Again, very many thanks...

claremacrae avatar Oct 22 '22 19:10 claremacrae

Hi @alexthewilde - I hope you're OK. I just wanted to make sure you've seen my comments above...

claremacrae avatar Oct 24 '22 21:10 claremacrae

Here's the very simple obsidian-tasks-extended plugin to show how a Tasks Extension plugin can be written.

I see that the 'verify' GitHub Action failed. Looks like there are some ESLint issues????

I look into this.

What impact would it have on future refactoring of the Tasks code?

This change is orthogonal to the existing code, so it won't break any existing functionality.

alexthewilde avatar Oct 25 '22 09:10 alexthewilde

Test plugin

Here's the very simple obsidian-tasks-extended plugin to show how a Tasks Extension plugin can be written.

Thank you.

I was able to install a build of this PR in to the sample vault in the tasks repo - and to follow your instructions in the above example repo.

It ran with no errors, and will allow me to experiment with the changes, which is great.

This was my task block:

```tasks
group by happens
show do today button
```

Oddly, the new button was displayed on every task. I cannot see the logic error - but it would be good to understand the location of any problem, in case it affects the PR.

image

The feature's impact on future changes to the Tasks code

What impact would it have on future refactoring of the Tasks code?

This change is orthogonal to the existing code, so it won't break any existing functionality.

I meant the reverse. Whether I will be prevented from improving the code in the Tasks repo, such as by adding missing abstractions.

For example, your sample plugin contains this code:

  doTodayBtn.onClickEvent((event: MouseEvent) => {
      const updatedTask = new api.Task({
          ...task,
          scheduledDate: isToday ? null : today,
      });

      api.replaceTaskWithTasks({
          originalTask: task,
          newTasks: [updatedTask],
      });
  });

What if scheduledDate becomes a class, instead of a Moment?

What if api.replaceTaskWithTasks() changes the parameters it takes.

As soon as this feature is published, my hands will be tied from changing any current code that the PR makes accessible to people's extensions.

See Hyrum's Law..

claremacrae avatar Oct 25 '22 09:10 claremacrae

As soon as this feature is published, my hands will be tied from changing any current code that the PR makes accessible to people's extensions.

That isn't intended as a 'no' to this PR.

It's meant to be a request for advice and help on what would need to be done to which bits of Tasks to enable future changes to made safely.

For example, I presume it means removing all public data in classes that users might reasonably want to use in their extensions - replacing them with getters, and where relevant, setters.

And understanding how those settings might work as parameter types change over time.

Fields that store a Moment now will change to storing some TaskDate or similar class in future, for example.

claremacrae avatar Oct 25 '22 09:10 claremacrae

I see that the 'verify' GitHub Action failed. Looks like there are some ESLint issues????

I look into this.

Thanks. All are green now.

claremacrae avatar Oct 25 '22 09:10 claremacrae

(FYI I've ticked the 'My change requires a change to the documentation.' box in the original description... )

claremacrae avatar Oct 25 '22 09:10 claremacrae

For example, I presume it means removing all public data in classes that users might reasonably want to use in their extensions - replacing them with getters, and where relevant, setters.

That's right. Exposing an API comes with certain responsibilities. That's the trade-off I guess.

It would actually be better if the Tasks plugin exposed its API methods like this. This allows other plugin developers to access them like app.plugins.plugins['obsidian-tasks-plugin'].replaceTaskWithTasks().

Then the hook implementations wouldn't need to get the api object passed in anymore. That's anyway a workaround for the missing plugin API.

alexthewilde avatar Oct 25 '22 09:10 alexthewilde

Oddly, the new button was displayed on every task. I cannot see the logic error

There's no logic error. Displaying the button on every task is intended. You can explicitly control this via show do today button for your query.

alexthewilde avatar Oct 25 '22 09:10 alexthewilde

Oddly, the new button was displayed on every task. I cannot see the logic error

There's no logic error. Displaying the button on every task is intended. You can explicitly control this via show do today button for your query.

What's the point of the button then?

claremacrae avatar Oct 25 '22 09:10 claremacrae

For example, I presume it means removing all public data in classes that users might reasonably want to use in their extensions - replacing them with getters, and where relevant, setters.

That's right. Exposing an API comes with certain responsibilities. That's the trade-off I guess.

Good, we are agreed.

It would actually be better if the Tasks plugin exposed its API methods like this. This allows other plugin developers to access them like app.plugins.plugins['obsidian-tasks-plugin'].replaceTaskWithTasks().

Then the hook implementations wouldn't need to get the api object passed in anymore. That's anyway a workaround for the missing plugin API.

I don't understand this at all. I mean, I don't understand in what way the Tasks code would need to change in order to do what you suggest.

claremacrae avatar Oct 25 '22 09:10 claremacrae

I don't understand this at all. I mean, I don't understand in what way the Tasks code would need to change in order to do what you suggest.

Actually, I now kind of do get it. Does all the use of any again hide the fact that if the interface of a Task changes, it will break client code.

Should we be looking to expose some kind of .d.ts (?) file to publish an interface.

claremacrae avatar Oct 25 '22 10:10 claremacrae

Should we be looking to expose some kind of .d.ts (?) file to publish an interface.

Exposing types doesn't help with exposing the actual methods. You actually need to make each method that you want to expose a member of the main class.

To be frank, what we're talking about here are developer ergonomics. It's not really that important right now. Either way, if you change the plugin's API methods (regardless of how they're being exposed), then this needs to be communicated to dependent developers, e.g. via "breaking change" in your Changelog.

alexthewilde avatar Oct 25 '22 10:10 alexthewilde

Oddly, the new button was displayed on every task. I cannot see the logic error

There's no logic error. Displaying the button on every task is intended. You can explicitly control this via show do today button for your query.

What's the point of the button then?

To toggle the scheduled date to today or null i.e. reset it.

I might be the only person on the planet who needs this. But now I'm able to build it myself, instead of begging you to add this button to the Tasks core plugin :)

alexthewilde avatar Oct 25 '22 10:10 alexthewilde

To be frank, what we're talking about here are developer ergonomics. It's not really that important right now. Either way, if you change the plugin's API methods (regardless of how they're being exposed), then this needs to be communicated to dependent developers, e.g. via "breaking change" in your Changelog.

Indeed.

To clarify, the reason I'm persisting with this is that my very strong philosophy is to work hard to minimise the number of breaking changes.

I do this in all projects that have external users, but especially in Tasks, as it has around 175,000 downloads now, and I know for sure that many people will take advantage of your new feature when it is released.

That focus is mainly for the benefit of users - but it's also for my own benefit. When I took over looking after Tasks, I completely under-estimated how much of my own time would go just on user support. And a breaking change would undoubtedly worsen that.

If someone shares a Tasks extension that a bunch of people use, that is later broken by a Tasks update, my support load is guaranteed to increase.

So I am after a better backwards-compatibility story than "log a breaking change in the changelog".

claremacrae avatar Oct 25 '22 10:10 claremacrae

@claremacrae that makes absolute sense. Btw thank you for maintaining the plugin! I love the simplicity of letting me create tasks (instead of task notes) while I'm in the flow of writing.

Regarding backwards-compatibility: right now, only replaceTaskWithTasks gets exposed. Unless you plan to change this method frequently, you shouldn't have to worry about too many regressions.

The more API methods you expose however, the more things can break. So you should indeed be careful here.

alexthewilde avatar Oct 25 '22 10:10 alexthewilde

@alexthewilde Thank you, that helps a lot.

The thing I'm missing, though, is that you say only replaceTaskWithTasks gets exposed.

But it's actually the Task class that I'm concerned about. It is a monster. What am I not understanding here, about why future changes to it would not be a problem for users?

Another example, what if someone wanted to change the recurrence rule for a task. That's another class....

claremacrae avatar Oct 25 '22 10:10 claremacrae

But it's actually the Task class that I'm concerned about. It is a monster. What am I not understanding here, about why future changes to it would not be a problem for users?

Yes indeed. Changes to the Task model (not necessarily the class though) would affect everyone who uses it in an extension.

I think it depends on how stable the plugin is. If you plan on turning everything upside down, then I'd certainly not build something on top of it!

alexthewilde avatar Oct 25 '22 11:10 alexthewilde

I think it depends on how stable the plugin is. If you plan on turning everything upside down, then I'd certainly not build something on top of it!

I'm still learning about the code, and nothing in the following is cast in stone or even certain to happen...

One of the main reasons that user support takes time on this plugin is because there are some missing abstractions that make it hard than people assume. So I keep answering the same questions over and over again (taking time away from development)

Some examples - a far from complete list:

  • I want to use custom formats for the due date, etc in my tasks.
    • Sorry no. There is no abstraction for date data (Moment is used directly) so implementing that currently would be a ton of work.
  • I don't like emojis, I want to use [dataview format, or one of a myriad of other formats]...
    • Sorry no. All the parsing of Task lines is in the already massive Task class. There should be abstractions for the various different formats.
  • My task is in an H3, I want to search text in its parent H1, H2, H3
    • Sorry no. Tasks are stored in a flat list, and they only know about their immediate previous heading. We should have an abstraction for sections in markdown files.
  • I want to search nested tasks - or show nested/indented tasks
    • Sorry no. Tasks are stored in a flat list

I am really good at refactoring existing code safely. I can do all these things. I just need some days of uninterrupted spare time to focus on these things, and the irony of so much time going on user support is that it makes that harder...

I'm even considering reworking all the parsing code to base it on the dataview API.

claremacrae avatar Oct 25 '22 12:10 claremacrae

OK - on re-reading the above, I think that the answer is that I am happy to commit to keeping working any extensions that are written against Tasks 1.x...

And in the documentation I will say to expect that they will need to be updated when Tasks 2.x is released. That there is no compatibility guarantee that extensions will continue to work across major version changes.

This all means that this PR will need to include a smoke test to allow easy manual testing that an extension still works with the current (v.1.x) code.

Here is the current smoke testing page: obsidian-tasks/resources/sample_vaults/Tasks-Demo/Manual Testing/Smoke Testing the Tasks Plugin.md

So the smoke test should look something like:

  • Load the page blah
  • Switch to Reading view
  • Confirm that [some customisation is visible]
  • Confirm that nothing no errors were written in the console

As well as being useful for smoke testing, the new page in the sample vault will be useful for people to test out and experiment with the new capability.

claremacrae avatar Oct 25 '22 12:10 claremacrae

This all means that this PR will need to include a smoke test to allow easy manual testing that an extension still works with the current (v.1.x) code.

Maybe I'll find some time on the weekend. Do you see yourself capable of writing smoke tests?

alexthewilde avatar Oct 25 '22 12:10 alexthewilde

Maybe I'll find some time on the weekend. Do you see yourself capable of writing smoke tests?

Yes, of course I can. Time permitting, of course.

This one is a lot more work though, as it involves figuring out how to include a sample plugin inside this vault - and ensuring that when someone downloads the sample vault from the Artifacts list in the GitHub Actions log, it has the uptodate build of the extension plugin, with all its dependencies built.

So the bulk of the work here is not in writing the the 1 or 2 sentences in the smoke test, it is ensuring that it only takes someone a minute or so to run the smoke tests, from the download.

claremacrae avatar Oct 25 '22 12:10 claremacrae

Or maybe the extension demo/test plugin goes in another repo in the obsidian-tasks-group user, so that it can have its own Github Actions - and it's easy for people to fork it and modify it to their heart's content?

And then it is just installed inside the sample vault...

claremacrae avatar Oct 25 '22 13:10 claremacrae

@claremacrae I'm afraid that writing smoke tests will take me too much time figuring things out and I don't have that time really. Sorry that I can't be of help here. I was hoping that you could go forward with this task. If not, maybe somebody else from the community with more experience in that regard?

Btw. feel free to fork Obsidian Tasks Extended into your GitHub account if need be.

I will update the Docs on how to use the new hooks.

alexthewilde avatar Oct 29 '22 06:10 alexthewilde

Hi @alexthewilde No problem. I am definitely happy to carry on with it and get it to release.

As an aside, the reason I assigned a few people to their PRs was that I've created a personal board to keep track of the many things in flight here, and it helped to see names against the creators of the PRs... Nothing more than that.

claremacrae avatar Oct 29 '22 06:10 claremacrae

Btw. feel free to fork Obsidian Tasks Extended into your GitHub account if need be.

That's great - thank you!

I will update the Docs on how to use the new hooks.

Much appreciated.

claremacrae avatar Oct 29 '22 06:10 claremacrae

re: https://github.com/obsidian-tasks-group/obsidian-tasks/pull/1249#issuecomment-1290283445

specifically the following line

It would actually be better if the Tasks plugin exposed its API methods like this. This allows other plugin developers to access them like app.plugins.plugins['obsidian-tasks-plugin'].replaceTaskWithTasks()

I believe this is related to #1389.

BluBloos avatar Feb 04 '23 19:02 BluBloos

Implications for future changes to the Tasks code

What impact would it have on future refactoring of the Tasks code? If this were published, for example, would it prevent changes to the interface of the Task class? Asking because there is a lot of public data that I would want to protect at some point... I would hate to break a load of people's customisations - but would also prefer not to have our hands tied for future refactorings...

The above is now a solved problem!

As part of the new group by function facility, I've started publishing a documented set of properties that can be accessed on Tasks, and I will guarantee these going forwards. They are mostly getters that are separate from the stored data, so do not prevent future refactorings...

https://publish.obsidian.md/tasks/Scripting/Task+Properties

For now, these properties are read-only, and we would need to define a convention for updating task objects that is independent of the storage.

But with this, in conjunction with the recently added API, I can see that adopting the ideas in this PR could become very feasible in the foreseeable future. https://publish.obsidian.md/tasks/Advanced/Tasks+Api

claremacrae avatar Jun 15 '23 18:06 claremacrae

Setting to draft status any PRs which are not currently mergable, but will be useful in future.

claremacrae avatar May 07 '24 19:05 claremacrae