Skip to content
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

(Feature) Unscheduled tasks #43

Closed
Blondwolf opened this issue Jan 31, 2024 · 17 comments · Fixed by #45
Closed

(Feature) Unscheduled tasks #43

Blondwolf opened this issue Jan 31, 2024 · 17 comments · Fixed by #45

Comments

@Blondwolf
Copy link

Blondwolf commented Jan 31, 2024

Context: .ics, .icalendar


First, thank you a lot. I was searching for something to synchronize my tasks with my calendar and this plugin is just perfect.

This would be even more perfect if the .ics file would also be populated with unscheduled tasks as "todo". I've try with both .ics and .icalendar format but does not seem to load the tasks.

This is compatible with .ics and .icalendar format:

BEGIN:VTODO
SUMMARY:Name
DESCRIPTION:Blablabla
STATUS:NEEDS-ACTION
PRIORITY:0
SEQUENCE:0
END:VTODO

This could be then enabled with a boolean switch in settings.

Thanks in advance

@Blondwolf
Copy link
Author

According to code Tasks.ts

You could add a parameter in constructor and then adapt a bit:

bool scheduledTask = (dateMatch === null)

Then use it to lead your process:

  if ( scheduledTask )
    const taskDates = getTaskDatesFromMarkdown(line);
  else
    const taskDates = [ ]; // or VTODO according to ics format?

Maybe there are other places that needs to be adapted.

Full example (not tested):

  const dateMatch = [...line.matchAll(dateRegExp)][0] ?? null;

  // Check wether we need to load dates (if it is a scheduled task or not)
  bool scheduledTask = (dateMatch === null)
  
  // Extract the Task data points from the matches
  // **Maybe need adaptation here**
  const taskStatus = getTaskStatusFromMarkdown(taskMatch?.groups?.taskStatus ?? '');

  // Task is done and user wants to ignore completed tasks. Bail
  if (taskStatus === TaskStatus.Done && ignoreCompletedTasks === true) {
    return null;
  }

  if ( scheduledTask )
    const taskDates = getTaskDatesFromMarkdown(line);
  else
    const taskDates = [ ]; // or VTODO according to ics format?

  const summary = getSummaryFromMarkdown(taskMatch?.groups?.summary ?? '', howToParseInternalLinks);

  // taskDates could be empty
  return new Task(taskStatus, taskDates, summary, fileUri);

@andrewbrereton
Copy link
Owner

Hi @Blondwolf

Great suggestion.

I think a setting is a great idea, default to off to replicate the existing behaviour.

If the setting is on, the code will append the VTODO block to the end of the iCalendar.

I'm not sure if I need to implement the RELATED-TO property in the VEVENT and VTODO blocks.

andrewbrereton added a commit that referenced this issue Feb 5, 2024
- Create new settign to enable todo items to be added to iCalendar.

- If enabled, after generating the VEVENT block, also generate a VTODO block.

- Support uid, summary, dtstamp and location just as we do during VEVENT. Add support for due and completed. Also add support for status proeprty based on the task status.

- Update README to explain new setting.
@andrewbrereton
Copy link
Owner

Hi again @Blondwolf. I've created a branch and pull request for this one.

#45

Please review and let me know your thoughts.

@Blondwolf
Copy link
Author

Blondwolf commented Feb 5, 2024

Ahah I was about to implement it from a fork of your projet. Great yeah, it looks nice. I like your code bro. It is readable.
As I am more a man of test, I need to run it quick locally. I'll come back to you.


Oh well this RELATED-TO field is very interesting.
That would be smart to use it to express dependency over tasks like that in the ical/ics (dependancy by identation):

  • A task
    • A dependant task

Probably not the scope for this ticket issue I think. We should open a new one if you want to work on this.

@Blondwolf
Copy link
Author

Blondwolf commented Feb 5, 2024

Okey so, what you've done is ok but you still need to modify a line to include the tasks from your filtering.
Because I don't get any results with unscheduled tasks. So your filter is still active. You are removing the undated elements.

Task.ts line 81:

// This task doesn't have a date. Bail.
if (dateMatch === null) {
    return null;
}

You need to include a variable based on the isIncludeTodo setting you've just added. So you need a new parameter for you TaskFinder. Something like: ignoreUnscheduledTasks.

Just ref the setting with the class parameter

this.taskFinder = new TaskFinder(this.app.vault, this.settings.howToParseInternalLinks, this.settings.ignoreCompletedTasks, this.settings.ignoreOldTasks, this.settings.oldTaskInDays, this.settings.isIncludeTodos);

Then use it:

constructor(vault: Vault, howToParseInternalLinks: string, ignoreCompletedTasks: boolean, ignoreOldTasks: boolean, oldTaskInDays: number, ignoreUnscheduledTasks: boolean) {
    ...
    this.ignoreUnscheduledTasks = ignoreUnscheduledTasks;
}

and propagate in Task.createTaskFromLine:

if (dateMatch === null && ignoreUnscheduledTasks) {

But then you need adaptation. That's why I proposed the code in previous post


Just a comment on your architecture, there are some stuff I don't really understand why you made it such like the custom Mains.ts. I am pretty new to obsidian coding though I have a lot of programming experience so I don't know really what's the standart. Some parts just feels a bit overengineered.

like createTaskFromLine would more belong to TaskFinder I think as you are still performing the text analysis. And so you don't have to pass all parameters again.

Probably it's better that I wait on this milestone to have the unscheduled tasks completed, then I'll propose a refactored version from my fork and pull request.

@andrewbrereton
Copy link
Owner

Hi @Blondwolf thanks for your input here.

Very good pickup regarding missing VTODO blocks that don't have a date.

I've committed a change to the PR to address this.

Commit that allows for inclusion of Tasks without dates: ba0ab78

Whole PR: https://github.com/andrewbrereton/obsidian-to-ical-plugin/pull/45/files

I've got a test Obsidian vault with files and tasks. The generated iCalendar file now includes tasks without dates as VTODO items.

@andrewbrereton
Copy link
Owner

Regarding architecture, this is my first Obsidian plugin so I don't know if I'm doing things the right way or not. The Obsidian devs reviewed it, and gave some suggestions, and approved it so it can't be tooooo bad. It has grown organically though, so I will do a review at some stage. I'd like to have unit tests in place though so I can make changes with more confidence.

In Main, start() is just the main entrypoint. I know the Obsidian sample plugin treats Main similar to how I treat ObsidianIcalPlugin. However I wanted ObsidianIcalPlugin to only deal with the Obsidian integration, and Main to be the entrypoint to the business logic of the plugin.

createTaskFromLine() is a factory method to generate Tasks from strings. I agree that passing settings around like that is not ideal.

Once these current issues/PRs are merged, I plan on storing the settings in a singleton similar to the Logger class. This will fix two issues:

  1. I don't need to pass settings throughout the stack anymore, and
  2. I don't need to ensure that when settings are changed in the UI, the changes propagate through the classes (probably an existing bug).

@Blondwolf
Copy link
Author

Please don't get me wrong, the code is pretty clean. Furthermore if it's your first one. Anyway it's nearly impossible to implement the last version directly :p So you did right by implementing it somehow first.

I have something to say for each subject so I opened new tickets.

To keep subject on unscheduled tasks, let me just review it again. I need to quick adapt because I don't use Bun ahah

@Blondwolf
Copy link
Author

Blondwolf commented Feb 7, 2024

There is still a problem. You have doubles. events are added as todos as well.
I mean, it depends on what we want to achieve. We could also define many modes or another setting to define if events are added to todos as well.

For this:

  • Task
  • Planned event 📅 2024-02-06
  • Finished event 📅 2024-02-05 ✅ 2024-02-05

I have those 5 where I should only have 3. 1 Task and 2 events:

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Andrew Brereton//obsidian-ical-plugin v1.13.0//EN
X-WR-CALNAME:Obsidian Calendar
NAME:Obsidian Calendar
CALSCALE:GREGORIAN
BEGIN:VEVENT
UID:9cee450e-863e-42a9-ac5f-dd550629ffa1
DTSTAMP:20240206T000000
DTSTART:20240206
SUMMARY:🔲 Planned event
LOCATION:ALTREP="obsidian://open?vault=Obsidian%20Plugins&file=Test.md":obsidian://open?vault=Obsidian%20Plugins&file=Test.md
END:VEVENT
BEGIN:VEVENT
UID:67573e11-e4d7-471c-876c-9920dd8f15e8
DTSTAMP:20240205T000000
DTSTART:20240205
SUMMARY:✅ Finished event
LOCATION:ALTREP="obsidian://open?vault=Obsidian%20Plugins&file=Test.md":obsidian://open?vault=Obsidian%20Plugins&file=Test.md
END:VEVENT
BEGIN:VTODO
UID:f91a6e2c-3f49-4605-bb0c-a42d016da087
SUMMARY:🔲 Task
STATUS:NEEDS-ACTION
END:VTODO
BEGIN:VTODO
UID:dff64330-a5fe-4612-ae35-8f5183746dba
SUMMARY:🔲 Planned event
DTSTAMP:20240206T000000
DUE;VALUE=DATE:20240206
STATUS:NEEDS-ACTION
END:VTODO
BEGIN:VTODO
UID:50baad81-e4f0-4abf-8304-1d5f34ec76d1
SUMMARY:✅ Finished event
DTSTAMP:20240205T000000
DUE;VALUE=DATE:20240205
COMPLETED;VALUE=DATE:20240205
STATUS:COMPLETED
END:VTODO
END:VCALENDAR

@andrewbrereton
Copy link
Owner

I've implemented it so that every task with a date is a calendar event AND a todo, and every task without a date is only a todo.

I mean, it depends on what we want to achieve.

This is your feature request. What are your requirements?

We could also define many modes or another setting to define if events are added to todos as well.

Does your use-case require this?

@Blondwolf
Copy link
Author

You are too kind. I appreciate but I think that it is better to have something that fulfill the majority of people requirements. Not just mine.

What are your requirements?

Mine is event OR todo separated from each other. Well in fact my first main requirement is already completed by the plugin itself.

Basically I use it to send the .ics directly to my Radicale server. Via a plugin I made for triggering changes on the .ics file, then APIRequest plugin to send the trigger to a NodeRED server which accept my API request, read the .ics file and send it to Radicale via a PUT on caldav.

Having the possibility to synchronize the tasks as well would be nice.

We could also define many modes

So it would be a good idea to define another setting to specify if you want your event to be added as todo as well.
Or you could use an enum as setting (or a number if not allowed) to specify between 3-4 differents modes. Maybe it is better to have only 2 boolean if you are not so experienced.

@andrewbrereton
Copy link
Owner

Thank you for explaining your use-case, it is helpful. I'm glad to hear that your main requirement is completed. Throughout my life, something I try to avoid doing is building solutions for the assumed silent audience. Oftentimes I have found that it makes code more complex, and even more often, the assumed use-case does not exist in reality. (i.e.: ASSUME: makes an ASS of U and ME 😄). This is why I am a bit hesitant to implement something for a perceived problem.

With that said, I think it is rather simple to add a new setting which asks the user to decide if they want tasks with dates added as a TODO, or just tasks without dates added as a TODO. And again quite simple to consider this setting when building the iCalendar file. For example:

If Add tasks as TODO items to your calendar is true then show this setting:

Do you want to include only tasks without dates as TODO items, or all tasks?

Options:

  • Only tasks without dates (default)
  • All tasks

@Blondwolf
Copy link
Author

This is a good YAGNI feeling that you have. You should indeed always ask yourself if this is necessary.

Furthermore comparing to old softwares architectures (used to be closed, non-standardized), where we have now plugins, API, and more standards, that leads to a way better program interoparability. Simply say it is way easier to make softs communicate so enhance modularity.

In this case, this would be a bit hard to add a wrapper or an extension working with your plugin. Well it would possible to re-read the entire calendar file but that would be a bit an overkill. In this case, you are indeed generating the .ics file so it still make sens for me.

@andrewbrereton
Copy link
Owner

What are your thoughts on this issue? Should I:

  1. Mark it as done and close it,
  2. Add the additional setting Do you want to include only tasks without dates as TODO items, or all tasks?, or
  3. Something else.

@Blondwolf
Copy link
Author

Egoistically I would say 2)

But please, this is not urgent, so prioritize your needs

@andrewbrereton
Copy link
Owner

This has been in 1.17.0

@Blondwolf
Copy link
Author

Blondwolf commented Mar 21, 2024

I need to test it more but it looks damn awesome. Thank you a lot. Sorry I haven't been able to help you since.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants