-
-
Notifications
You must be signed in to change notification settings - Fork 232
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
Add hooks to let other plugins extend the Tasks plugin. #1249
base: main
Are you sure you want to change the base?
Conversation
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... How to test itI notice you haven't filled in the testing section. How are you imagining I might test it? GitHub ActionsI see that the 'verify' GitHub Action failed. Looks like there are some ESLint issues???? Implications for future changes to the Tasks codeWhat 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 Again, very many thanks... |
Hi @alexthewilde - I hope you're OK. I just wanted to make sure you've seen my comments above... |
Here's the very simple obsidian-tasks-extended plugin to show how a Tasks Extension plugin can be written.
I look into this.
This change is orthogonal to the existing code, so it won't break any existing functionality. |
Test plugin
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:
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. The feature's impact on future changes to the Tasks code
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 What if 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.. |
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 |
Thanks. All are green now. |
(FYI I've ticked the 'My change requires a change to the documentation.' box in the original description... ) |
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 Then the hook implementations wouldn't need to get the |
There's no logic error. Displaying the button on every task is intended. You can explicitly control this via |
What's the point of the button then? |
Good, we are agreed.
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 Should we be looking to expose some kind of |
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. |
To toggle the 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 :) |
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 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 The more API methods you expose however, the more things can break. So you should indeed be careful here. |
@alexthewilde Thank you, that helps a lot. The thing I'm missing, though, is that you say only But it's actually the Another example, what if someone wanted to change the recurrence rule for a task. That's another class.... |
Yes indeed. Changes to the 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 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. |
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: So the smoke test should look something like:
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. |
Maybe I'll find some time on the weekend. |
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. |
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 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. |
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. |
That's great - thank you!
Much appreciated. |
re: #1249 (comment) specifically the following line
I believe this is related to #1389. |
The above is now a solved problem! As part of the new 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. |
Setting to draft status any PRs which are not currently mergable, but will be useful in future. |
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
Renders as
Implemented in new plugin
Types of changes
Changes visible to users:
fix
- non-breaking change which fixes an issue)feat
- non-breaking change which adds functionality)feat!!
orfix!!
- fix or feature that would cause existing functionality to not work as expected)docs
- improvements to any documentation content)vault
- improvements to the Tasks-Demo sample vault)Internal changes:
refactor
- non-breaking change which only improves the design or structure of existing code, and making no changes to its external behaviour)test
- additions and improvements to unit tests and the smoke tests)chore
- examples include GitHub Actions, issue templates)Checklist
yarn run lint
.Terms