Core Rewrite [RFC]

[diegogangl]

0.4 brought the port to Gtk3, Python3, GtkApplication, GtkMainWindow, GActions and Meson.
0.5 is bringing Lxml, a new taskview and a new file format.

This ticket is the result of the notes I’ve been taking over many months, a bird's eye view and discussion of the next big refactor(s). This should start in 0.6 but will likely take a few releases to come together.

Also pinging: @Neui and @jaesivsm for comments

The targets for this are:

• Improve Performance
• Improve Maintainability
• Have more flexibility in the UI design
• Pay off the accumulating technical debt

Comments, ideas, any feedback welcome!

Problems

GUI

TreeViews have fallen out of favor in most Gnome Apps since the days of Gtk2.

One of the problems with TreeViews is that their rendering method (cell renders) is different from regular widgets, which means they can’t use CSS or be animated. We are also severely constrained in the UI we can design for task/tag rows.

I’m also not very clear on how TreeViews fit with adaptive GUIs (which we may want to do at some point).

Finally we will eventually have to move away from Treeviews. At the very least for column views in Gtk4.

And, to be clear, it also does not mean that we are removing treeviews and combo boxes from GTK 4—it is too late for that, and they are still used in many places inside GTK. That may be a GTK 5 goal.
The way forward is using list boxes (specially for our use case). However that brings a new problem: we can’t use the TreeModel liblarch is based on with list boxes.

This was partially prophesied in the old GTG mockups: https://wiki.gnome.org/Apps/GTG/Design

Liblarch

According to @ploum:

1. I started to write the whole gtktreeview from scratch until I realised that it was not always easy to figure out which tasks were displayed.
2. I had the idea to put an abstraction layer between the data-model and the view. That abstraction was called "filteredtree".
3. As more and more usecases appeared, filtered tree became a huge beast with a lot of bug. Solving a bug was creating 10 more.
4. I decided to abstract the whole thing into a library in order to separate those bugs from bugs in GTG itself.
5. As a library, liblarch has one major advantage: I could write unit tests. Liblarch was, at some point, nearly fully test covered.

Liblarch also includes support for multiple parents for a task (or node). The use case is subtasks that are blocked by more than one task. However, this could also be implemented by adding a “blocked_by” field and linking tasks by their IDs.

The biggest problem with liblarch is performance. Most of GTG performance issues can be traced back to liblarch, in particular the sorting and refiltering methods.

The second problem is maintainability. Liblarch adds a lot of code on our shoulders for the purpose of being a library, making TreeViews more convenient and supporting multiple parents per task. However:
GTG remains the only user of liblarch, and that’s unlikely to change (given where the general trend in UIs is going)
We and the general design of GNOME are moving away from Treeviews
We don’t use multiple parents (and have other possible solutions)

Inside GTG

Unit testing GTG is complicated because most classes are tightly coupled. Pretty much everything depends on the requester, and the requester depends on the datastore which initializes the backends and config system. So if you want to test a particular class you end up pulling everything.

We are also not properly using signals in Gtk, which leads to a lot of callback functions bouncing all over and tight coupling between classes.

Tags

Tags are designed to be created and deleted on the fly, but this conflicts with the idea of having persistent tag settings (color, icon, etc) as well as having children. This makes things like renaming rather hacky because the tag name acts as an ID, the tag being renamed has to be deleted and have its settings copied to a new one.

The sidebar is a single Treeview. “All Tasks”, “Tasks with no tags”, “Saved Searches” and even the separator line are implemented as special tags that get created on runtime abusing the attributes system (technically a dict).

Every method that deals with the list of tags has to make special exceptions for tags that have a “special” or “query” attribute.

Proposal

Move the models to GListStores. Then create wrapper classes that will take care of manipulating the models, and de/serializing using lxml. Make a new class for Saved Searches (SavedSearch) to separate them from tags. Simplify classes and their responsibilities to make unit testing easier. Use signals along with these changes to reduce coupling. Maybe even get rid of the requester eventually? (out of the scope for this)

• SearchList: Contains SavedSearch
• TaskList: Contains Task
• TagList: Contains Tag

Tags, Tasks and Saved Search classes should only be responsible for themselves and have methods that manipulate their own data. That means no new subtasks from inside a task, no save methods on tags, etc.

New subtasks (for example) should be handled by the TaskList class, which takes care of inheriting tags/repeat/etc and then parenting the tasks.

Likewise, The *List classes should only deal with their own model and data.
The DataStore should be in charge of reading the xml files and passing the lxml elements into these classes so they can populate themselves.

Saving would work the other way around, *List classes would call a method to get an lxml element from a Task, Tag, etc. and gather them into their own root tags. All of those would be returned back to the DataStore which takes care of writing the xml with all those root elements (taskList, tagList, etc). With the new file format, everything is in one place so it makes sense to have only one save function. Saving can then be triggered by signals.

We should do some profiling to see if saving takes too long, and whether it’s worth writing it on a separate thread.

Roadmap

Each one of these steps is designed to be a small branch that can be tested and merged back into master somewhat quickly. This will also take more than one release, so this roadmap is designed to add the pieces gradually without affecting users and contributors.

This also means we can add smaller features and keep releasing while we work on this (and avoid dev hell). None of the phases can block a release, except phase 5.

Phase 0: Cleanup and Setup

• Find and remove dead code from Tag and Task
• Remove setters and getters that only have one line. Those that actually do something should use the @property decorator
• Add Developer Console plugin (this will make life much easier)
• Remove the Borg wrapper in Datastore. The requester has access to the full datastore, and backends can just import the requester to workaround this class, there’s no point in faking private methods
• Convert the Requester class into a module. The requester is a class with no state, so it can be simplified into a module with functions and variables. This way all we have to do is import the requester and we don’t have to pass it around all the time (delayed until the other classes are in place)
• Move tag writing into local backend
• Remove config options that aren’t used anymore
• Use signals in the new Taskview (delayed: This could use some design, so we can use signals everywhere instead)
• Use __slots__ in Date class

Phase 1: Saved Searches

• Add a new module and class for saved searches
• Add the list model for Saved Searches
• Write unit tests

Phase 2: Porting Tag

• Add a new class for tags (Called tag2)
• Add the list model for (TagList)
• Write unit tests

Phase 3: Porting Task

• Add a new class for tasks (Called task2)
• Add the list model for (TaskList)
• Write unit tests

Phase 4: Read and save

• Make a new DataStore2 module and class
• Plug all the new classes into the new datastore
• Read and save XML through the new datastore
• Plug other backends (port code over from the old datastore)
• Write unit tests for the new datastore

Phase 5: GUI Improvements

This doesn’t include any big UI changes. New features could happen if they are easy enough to implement (or happen “naturally”).

• Replace the TreeView in the sidebar with a Box
• Add buttons for all tasks and tasks with no tags
• Add a listbox for saved searches
• Add a listbox for tags
• Replace treeviews in the task browser for with listboxes

Phase 6: Removing the old code

• Rename Task2 and Tag2 to Task and Tag
• Delete everything that’s no longer used
• Party Time. Excellent!

[diegogangl]

Forgot to also ping @zeddo123 for comments :)

[Neui]

I saw this, but forgot to look more closely and comment. I'm not an experienced architect or something. It's also currently midnight.

GUI

I haven't really looked into TreeView and alternatives yet, but I think we should at least keep the tree display, since it shows the "relations".

Looking at GTK4 it still has TreeView, but also a simplier list-based that is supposed to be more customizable, but need additional work to emulate tree views.
I would be OK for using the List, but then it means we would need to re-implement the tree view.

If we are already talking about adaptive UI, we could also look into libhandy, but since I don't think it dramatically changes the code, I wouldn't consider it for this issue.

Liblarch

I'm not sure about merging liblarch into GTG.
Sure, we're the only user, but they're separate, and fits being a library in my opinion.

Inside GTG

I agree about using signals more than callbacks.

I don't really know what the requester and datastore exactly do, so I can't really comment on that.

Tags

I haven't really looked into yet, but separating tags from non-tags sounds good.
From what I understand putting them all inside a list is a bad idea. Yes, but when only using such list to display it (NOT to expose to other code) seems fine to me, as long we can differentiate it.
Alternatively make the GUI be different (2 boxes?).

About the tags, I think letting them have attributes is OK.
I would have suggested using UUIDs and the name is just a attribute, but then I wonder about when combining sources together, where the name are also used as an identifier.
I am not sure what to do.

I am also not sure about tags having children. I don't think it is used that often, but I am not sure if there are "valid" usecases for it to keep.

Proposal

Sounds fine overall.

From what I understand, serialization (and probably deserialization) would be done in the Lists themselves, which I am unsure about.
I would keep (de)serialization separate from the Lists and let the DataStore (or whatever would handle it) do it.
If for example the CalDav backend would use this, it would need to first convert to lxml, then to its format rather than skipping the lxml step.
Possibly provide some "common" methods to convert to lxml for backends to use if they want (maybe profiles could be realized by a 3rd party backend?)

I would definitively suggest moving "long-running" tasks to their thread so the UI is at least responsive, which is in my opinion important.
Having "frozen"/non responding application is bad.

Roadmap

For Phase 0, not sure about the Borg and requester stuff, since I don't know what it does/is for.

In phase 5 I am worried a little about replacing it with listboxes. It would imply we need to do the tree stuff again (unless there is something premade).

One thing is that they seem a bit ambitious. It seems a lot to do, and we need people not loosing interest too fast and not go "not too much time currently".

Also, we need extensive testing, especially when we're switching to the new implementation (Primarily phase 6 from what I can see).

[diegogangl]

I saw this, but forgot to look more closely and comment. I'm not an experienced architect or something. It's also currently midnight.

No problem, thanks for commenting :D

I haven't really looked into TreeView and alternatives yet, but I think we should at least keep the tree display, since it shows the "relations".
I would be OK for using the List, but then it means we would need to re-implement the tree view.

Yep, that's the idea. We need to replicate the indent and showing/hiding but Gnome Todo and Planner already do this, so it is definitely doable.

If we are already talking about adaptive UI, we could also look into libhandy, but since I don't think it dramatically changes the code, I wouldn't consider it for this issue.

Libhandy (soon to be libadwaita?) doesn't provide anything for trees, only listboxes:
https://gnome.pages.gitlab.gnome.org/libhandy/doc/1.0/

Paving the way to adaptive UIs is def part of this proposal though.

Liblarch

I'm not sure about merging liblarch into GTG.
Sure, we're the only user, but they're separate, and fits being a library in my opinion.

This isn't about merging liblarch, but rather replacing it with a simpler system. Liblarch could still live in its repo.

I don't really know what the requester and datastore exactly do, so I can't really comment on that.

The datastore manages backends, while the requester is a neat API around it.

From what I understand putting them all inside a list is a bad idea. Yes, but when only using such list to display it (NOT to expose to other code) seems fine to me, as long we can differentiate it.
Alternatively make the GUI be different (2 boxes?).

Not sure I understand what you meant here

I am also not sure about tags having children. I don't think it is used that often, but I am not sure if there are "valid" usecases for it to keep.

I use this to organize tags in the sidebar (don't know if there are any extra features)

I would keep (de)serialization separate from the Lists and let the DataStore (or whatever would handle it) do it.

Yup, the datastore would do it.

If for example the CalDav backend would use this, it would need to first convert to lxml, then to its format rather than skipping the lxml step.

I don't quite understand this part. There's always a localfile backend, so the xml is always read and saved.

One thing is that they seem a bit ambitious. It seems a lot to do, and we need people not loosing interest too fast and not go "not too much time currently".

Indeed. However this can run parallel to other things in GTG all the way to phase 5, so there's little risk.

[jaesivsm]

GUI

TreeViews have fallen out of favor in most Gnome Apps since the days of Gtk2.
[...]

Sadly for the whole GUI part, as much as I've been tinkering with it and liblarch to try to get some performance improvements out of it, I am not at all knowledgeable on the matter (or on the current state of Gtk). I know that Gtk4 might be coming up but I'll trust you on that part.

Liblarch

As GTG is the sole project making use of liblarch I'm all in favor of supporting the minimum viable functionalities. Having it separated from the main code of GTG is good, not having to support feature nobody uses is also good.

Inside GTG

Unit testing GTG is complicated because most classes are tightly coupled. Pretty much everything depends on the requester, and the requester depends on the datastore which initializes the backends and config system. So if you want to test a particular class you end up pulling everything.

Although I did manage to work up some unittest on the caldav branch (very lite on the "unit" part) I'm agreeing with you. Testing shouldn't be the only purpose of that refactor but it's indeed needed.

Proposal

[...]

All in all : 👍

Roadmap

[...]

Phase 0: Cleanup and Setup

• Remove setters and getters that only have one line. Those that actually do something should use the @property decorator

In the same spirit I would also suggest to get rid of overriding __getattr__ when not needed (example). I saw it a lot in liblarch and it's mainly used to proxy attributes to objects attributes. Same thing, either denormalize it or replace it with a @property.

For objects like Date which are very often instantiated, we could also make use of __slots__ for performance improvement.

---

I would be happy to participate in that refactor. Especially since some of the points in the roadmap sounds within my reach :)

[diegogangl]

In the same spirit I would also suggest to get rid of overriding getattr when not needed (example). I saw it a lot in liblarch and it's mainly used to proxy attributes to objects attributes. Same thing, either denormalize it or replace it with a @Property.

Nice, will add that to the roadmap too

For objects like Date which are very often instantiated, we could also make use of slots for performance improvement.

Dude! Fantastic catch, we should definitely do that.

I would be happy to participate in that refactor. Especially since some of the points in the roadmap sounds within my reach :)

Welcome aboard :)
The only thing that's done is the dev console plugin

[nekohayo]

Overall the goals sound nice (from a cursory reading), particularly if you can indeed do it in modular refactorings that "work" even standalone, so that:

1. the main/master branch is always in a usable state;
2. you limit the mess of merge conflicts and divergence that a "huge branch" would involve;
3. you don't risk burning out or doing a Netscape. Refactoring is OK, but please don't fall into the trap of rewriting for the sake of rewriting, see this timeless piece of wisdom: https://www.joelonsoftware.com/2000/04/06/things-you-should-never-do-part-i/

I'm glad you have "Write unit tests" as a goal in every phase, as there definitely should be tests to cover extensively what you'll refactor! It wouldn't be fun to spend months of devhell depending on humans like me to retest everything and repeatedly reporting zombie issues each time.

As for liblarch, I would feel more at ease if you can do your invasive technological changes to GTG and to liblarch while preserving the existence of liblarch as a library that GTG uses, even if it's tailored to GTG; presuming @ploum's principles in issue #286 still hold true in this future architecture of yours, I think a big part of why liblarch exists as a standalone core and why it may be best to refactor & tune it (rather than abandoning it) is that it keeps you "honest" technically... As Lionel said last year:

"making liblarch a separated library allowed us to be very strict: no gross hack. We should define a clear API and use that API. We don't have the choice to workaround the API (which is what we always ended doing before liblarch)."

I fear that if we move away from liblarch, in X years someone is going to say "Oh gosh, everything is a big spaghetti inside GTG's core, we gotta split that out into a library or something!" (just like in organizations, there is always the back-and-forth between centralization and decentralization). Also, keeping it separate might (I presume) keep the GTG codebase more manageable for new/occasional contributors to GTG, and who knows, maybe projects like Planner could use liblarch too somehow (in a way liblarch could serve as bit as its own "libAdwaita")

While GTG is currently the only project known to use liblarch, it also means you don't have to guarantee the availability of old functionality in liblarch and can instead tailor it to your exact vision, so in practice it's not too limiting on that front, no?

Personally, I've often encountered the case where a task of mine blocks several others, so in an ideal world I'd love to have the ability for my tasks to have multiple parents, which is mostly a visual representation challenge (see issue #634).

[diegogangl]

I'm glad you have "Write unit tests" as a goal in every phase, as there definitely should be tests to cover extensively what you'll refactor! It wouldn't be fun to spend months of devhell depending on humans like me to retest everything and repeatedly reporting zombie issues each time.

Yes! One of the big ideas behind the rewrite is to make it easier (or even possible) to unit test. The new_core branch already adds ~36 new unit tests with nearly 100% code coverage: https://github.com/getting-things-gnome/gtg/compare/new_core

As for liblarch, I would feel more at ease if you can do your invasive technological changes to GTG and to liblarch while preserving the existence of liblarch as a library that GTG uses, even if it's tailored to GTG; presuming @ploum's principles in issue #286 still hold true in this future architecture of yours, I think a big part of why liblarch exists as a standalone core and why it may be best to refactor & tune it (rather than abandoning it) is that it keeps you "honest" technically... As Lionel said last year:

This rewrite doesn't touch liblarch at all. I mentioned in the chat that being "technically honest" should be the normal throughout all of GTG and not just the core, and making architectural decisions to discipline ourselves is a bad idea. In any case we can develop the new core completely in parallel (as I've been doing), and test it with unit tests and the developer console before plugging it into the UI.

I fear that if we move away from liblarch, in X years someone is going to say "Oh gosh, everything is a big spaghetti inside GTG's core, we gotta split that out into a library or something!" (just like in organizations, there is always the back-and-forth between centralization and decentralization). Also, keeping it separate might (I presume) keep the GTG codebase more manageable for new/occasional contributors to GTG, and who knows, maybe projects like Planner could use liblarch too somehow (in a way liblarch could serve as bit as its own "libAdwaita")

Ah man, Planner... that takes me back. I don't see how splitting things into a library solves spaghetti code, other than forcing you to rewrite the code. Which you can already do without splitting. I also don't see how this makes things more manageable for contributors, who would have to work back and forth with two different repos. Which parts of GTG are in liblarch, and which on GTG itself? It's not so simple. For instance, you still have GTG code sitting in liblarch: https://github.com/getting-things-gnome/liblarch/blob/master/liblarch/filters_bank.py

Personally, I've often encountered the case where a task of mine blocks several others, so in an ideal world I'd love to have the ability for my tasks to have multiple parents, which is mostly a visual representation challenge (see issue #634).

I'll reply in that issue so we don't crowd this one too much

[diegogangl]

Updated the roadmap section with the current status and changes on phase 4.

[leio]

As a couple quick comments:

• planner has no use of liblarch, as former is in C and latter is in pure python. There may be some opportunities for shared data models or rendering widgets (gantt), but then in some libegg/libhandy-style C API providing library with introspection support (to consume from python too).
• If you go all into gtk4, you'll have GtkFilterListModel and such to implement the filtering with, https://blog.gtk.org/2020/09/08/on-list-models/ https://developer.gnome.org/gtk4/stable/GtkFilterListModel.html
• GTK4 GtkColumnView can represent trees, just implemented differently, and everything is a widget (no cell renderers).

(feel free to poke me on IRC if there's follow-ups on these points here, as it's hard for me to notice and react to GH notifications)

[diegogangl]

@leio Gah, I came to comment about something else and just saw your comment. Sorry I took so long to reply!

planner has no use of liblarch, as former is in C and latter is in pure python. There may be some opportunities for shared data models or rendering widgets (gantt), but then in some libegg/libhandy-style C API providing library with introspection support (to consume from python too).

This could be interesting indeed. We had something like a gantt graph, in the calendar view plugin. Don't know how many widgets like we would really need though, something that looks like a calendar would be the most useful one I think

If you go all into gtk4, you'll have GtkFilterListModel and such to implement the filtering with, https://blog.gtk.org/2020/09/08/on-list-models/ https://developer.gnome.org/gtk4/stable/GtkFilterListModel.html

I already worked out some tests for that in #737 . And ranchester has a more advanced Gtk4 branch that includes those filters/sorters (https://github.com/ranchester2/gtg/tree/ng)

GTK4 GtkColumnView can represent trees, just implemented differently, and everything is a widget (no cell renderers).

Yup, saw that when I studied the docs. I still think we should move away from trees and explore other options (Also added a mockup in #737 )

[diegogangl]

Now that backends are back, I've been reading up on the code and writing down a few ideas.

Big question: Do backends need to be different from plugins?

• The local backend is gone now in the new core, since the datastore takes care of writing the xml. All the special handling for the local backend is not needed anymore. Writing, reading, backing up, etc. is handled by the datastore. And since we can instantiate as many DS as we want we can also do some of the things the local backend was slated to do, like switching profiles on the fly for instance.
• Eventually we want to have a way for users to install third party plugins. What if we also want to allow third party backends? Would we end up duplicating code? (since their paths are so apart)
• On the other hand, backends have a queue-system to write to tasks without overwriting each other. What if we wanted plugins that wrote things to tasks?
• Basically what I'm saying is that backends could just be plugins that sync tasks. And we can consolidate everything under that roof.
• Some backend callbacks could be replaced by signals. We can have the backends connect to them, rather than mantain a list of enabled backends and run callbacks through each one of them. (like in the quit() process)
• Utilities like periodic import could be part in the plugin api.
• In any case, we should definitely get rid of the "Auto UI" system (the parameter dicts). It adds maintenance overhead and there's no way to cover all possible situations in a way that fits every possible backend. It also leaves no room for backends to organize settings in a way that makes sense to them. Not to mention that it needs to be updated to at least Gtk3 standards. It would be much easier to pass a Gtk container to a callback function and let it fill the container with widgets.

Pinging @jaesivsm since I think you know the backends code better than anyone ATM