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

Introduce the StoreItem base class #1158

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 41 additions & 6 deletions GTG/core/base_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,52 @@
import logging

from lxml.etree import _Element
from typing import Dict, List, Optional, TypeVar, Generic, Protocol
from typing import Dict, List, Optional, TypeVar, Generic
from typing_extensions import Self


log = logging.getLogger(__name__)

S = TypeVar('S',bound='Storeable')
S = TypeVar('S',bound='StoreItem')

class StoreItem(GObject.Object):
"""Base class for items in BaseStore."""

__gtype_name__ = 'gtg_StoreItem'


def __init__(self,id: UUID):
self.id: UUID = id
self.parent: Optional[Self] = None
self.children: List[Self] = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.children: List[Self] = []
self.children: list[Self] = []

Now that Python 3.8 is gone, we can write code that targets 3.9, which means we no longer need typing.List and the like.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. (We should also update the readme and every other reference to python version.)

super(StoreItem, self).__init__()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
super(StoreItem, self).__init__()
super().__init__()

Bad habits form Python 2 still in the code base :) Let's fix them as we see them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.



@GObject.Property(type=int)
def children_count(self) -> int:
"""Read only property."""
return len(self.children)


@GObject.Property(type=bool, default=False)
def has_children(self) -> bool:
return len(self.children) > 0


def get_ancestors(self) -> List[Self]:
"""Return all ancestors of this tag"""
ancestors: List[Self] = []
here = self
while here.parent:
here = here.parent
ancestors.append(here)
return ancestors


def check_possible_parent(self, target) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def check_possible_parent(self, target) -> bool:
def is_parent_of(self, target) -> bool:

and inverse the conditions below.

I think it might be clearer to read if not foo.is_parent_of(bar): rather than if foo.check_possible_parent(bar):?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the name is not perfect. However, is_parent_of wouldn't correctly describe the method (e.g., you can not parent an item to itself.) I used is_parentable_to in the new version. I think it also reads much better than the original one.

"""Check for parenting an item to its own descendant or to itself."""
return self != target and self not in target.get_ancestors()

class Storeable(Protocol[S]):
id: UUID
parent: Optional[S]
children: List[S]


class BaseStore(GObject.Object,Generic[S]):
Expand Down
24 changes: 3 additions & 21 deletions GTG/core/tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from lxml.etree import Element, SubElement, _Element
from typing import Dict, List, Set, Optional

from GTG.core.base_store import BaseStore
from GTG.core.base_store import BaseStore, StoreItem

log = logging.getLogger(__name__)

Expand All @@ -40,26 +40,23 @@ def extract_tags_from_text(text):
return re.findall(r'(?:^|[\s])(@[\w\/\.\-\:\&]*\w)', text)


class Tag(GObject.Object):
class Tag(StoreItem):
"""A tag that can be applied to a Task."""

__gtype_name__ = 'gtg_Tag'

def __init__(self, id: UUID, name: str) -> None:
self.id = id
self._name = name

self._icon: Optional[str] = None
self._color: Optional[str] = None
self.actionable = True
self.children: List[Tag] = []
self.parent = None

self._task_count_open = 0
self._task_count_actionable = 0
self._task_count_closed = 0

super(Tag, self).__init__()
super(Tag, self).__init__(id)


def __str__(self) -> str:
Expand All @@ -80,11 +77,6 @@ def __eq__(self, other) -> bool:
return self.id == other.id


@GObject.Property(type=int)
def children_count(self) -> int:
"""Read only property."""

return len(self.children)


@GObject.Property(type=str)
Expand Down Expand Up @@ -170,16 +162,6 @@ def set_task_count_closed(self, value: int) -> None:
self._task_count_closed = value


def get_ancestors(self) -> List['Tag']:
"""Return all ancestors of this tag"""
ancestors: List[Tag] = []
here = self
while here.parent:
here = here.parent
ancestors.append(here)
return ancestors


def get_matching_tags(self) -> List['Tag']:
"""Return the tag with its descendants."""
matching = [self]
Expand Down
18 changes: 6 additions & 12 deletions GTG/core/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

from lxml.etree import Element, _Element, SubElement, CDATA

from GTG.core.base_store import BaseStore
from GTG.core.base_store import BaseStore, StoreItem
from GTG.core.tags import Tag, TagStore
from GTG.core.dates import Date

Expand Down Expand Up @@ -78,19 +78,16 @@ class Filter(Enum):
DEFAULT_TITLE = _('New Task')


class Task(GObject.Object):
class Task(StoreItem):
"""A single task."""

__gtype_name__ = 'gtg_Task'

def __init__(self, id: UUID, title: str) -> None:
self.id = id
self.raw_title = title.strip('\t\n')
self.content = ''
self.tags: Set[Tag] = set()
self.children: List[Task] = []
self.status = Status.ACTIVE
self.parent: Optional[Task] = None

self._date_added = Date.no_date()
self._date_due = Date.no_date()
Expand All @@ -115,7 +112,7 @@ def default_duplicate_cb(t: Task):
raise NotImplementedError
self.duplicate_cb: Callable[[Task],Task] = default_duplicate_cb

super(Task, self).__init__()
super(Task, self).__init__(id)


@GObject.Property(type=bool, default=True)
Expand Down Expand Up @@ -617,11 +614,6 @@ def set_is_active(self, value) -> None:
self._is_active = value


@GObject.Property(type=bool, default=False)
def has_children(self) -> bool:
return bool(len(self.children))


@GObject.Property(type=str)
def icons(self) -> str:
icons_text = ''
Expand Down Expand Up @@ -702,7 +694,7 @@ def __hash__(self) -> int:
# STORE
# ------------------------------------------------------------------------------

class TaskStore(BaseStore):
class TaskStore(BaseStore[Task]):
"""A tree of tasks."""

__gtype_name__ = 'gtg_TaskStore'
Expand Down Expand Up @@ -1013,6 +1005,7 @@ def parent(self, item_id: UUID, parent_id: UUID) -> None:

# Add back to UI
self._append_to_parent_model(item_id)
assert item.parent is not None
item.parent.notify('has_children')


Expand All @@ -1023,6 +1016,7 @@ def unparent(self, item_id: UUID, parent_id: UUID) -> None:

# Remove from UI
self._remove_from_parent_model(item_id)
assert item.parent is not None
item.parent.notify('has_children')

super().unparent(item_id, parent_id)
Expand Down
18 changes: 1 addition & 17 deletions GTG/gtk/browser/sidebar.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,27 +590,11 @@ def drag_end(self, source, drag, unused):
source.get_widget().set_opacity(1)


def check_parent(self, value, target) -> bool:
"""Check for parenting a tag to its own descendant or to itself."""

if value == target:
return False

item = target
while item.parent:
if item.parent == value:
return False

item = item.parent

return True


def drag_drop(self, target, value, x, y):
"""Callback when dropping onto a target"""
dropped = target.get_widget().props.tag

if not self.check_parent(value, dropped):
if not value.check_possible_parent(dropped):
return

if value.parent:
Expand Down
18 changes: 1 addition & 17 deletions GTG/gtk/browser/task_pane.py
Original file line number Diff line number Diff line change
Expand Up @@ -612,28 +612,12 @@ def drop_enter(self, target, x, y, user_data=None):
return Gdk.DragAction.COPY


def check_parent(self, value, target) -> bool:
"""Check for parenting a task to its own descendant or to itself."""

if value == target:
return False

item = target
while item.parent:
if item.parent == value:
return False

item = item.parent

return True


def drag_drop(self, target, task, x, y):
"""Callback when dropping onto a target"""

dropped = target.get_widget().props.task

if not self.check_parent(task, dropped):
if not task.check_possible_parent(dropped):
return

if task.parent:
Expand Down
2 changes: 1 addition & 1 deletion docs/contributors/coding best practices.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ Modules should begin with the following header (updated to the current year):

```
# -----------------------------------------------------------------------------
# Gettings Things GNOME! - a personal organizer for the GNOME desktop
# Getting Things GNOME! - a personal organizer for the GNOME desktop
# Copyright (c) 2008-2022 - the GTG contributors
#
# This program is free software: you can redistribute it and/or modify it under
Expand Down
Loading