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

Mr custom #224

Closed
wants to merge 4 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 26 additions & 4 deletions hamster_lib/objects.py
Expand Up @@ -366,9 +366,10 @@ def __init__(self, activity, start, end=None, pk=None, description=None, tags=No
self.start = start
self.end = end
self.description = description
self.tags = set()
self.tags = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a specific reason for this change?

if tags:
self.tags = set(tags)
tags = set(tags)
self.tags = [Tag(name = x) for x in tags]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Whist you are correct that this should be a set of Tag instances the mistake is already in L356! The constructor should expect an iterable of such Tag instances, not strings. I created an issue about this.


@classmethod
def create_from_raw_fact(cls, raw_fact, config=None):
Expand Down Expand Up @@ -433,6 +434,16 @@ def at_split(string):
front, back = front.strip(), back.strip()
return (front, back)


def hashtag_split(string):
""" Return a list of tags marked with the hashtag.
"""
result = string.split('#')
result = [x.strip() for x in result]
result = [x.strip() for x in result if x != '']
return result


def comma_split(string):
"""
Split string at the most left comma.
Expand Down Expand Up @@ -468,7 +479,18 @@ def comma_split(string):
activity_name, back = at_split(rest)

if back:
category_name, description = comma_split(back)
# Check for an existing hashtag.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an interesting approach. The next iteration of hamster-lib will however refactor the entire 'raw fact' parsing machinery in total, which is why no effort has been made to take care of tags.

hashtag_pos = back.find('#')
if hashtag_pos >= 0:
category_name = back[:hashtag_pos].strip()
back = back[hashtag_pos:]
tag_string, description = comma_split(back)
tags = hashtag_split(tag_string)
else:
category_name, description = comma_split(back)
tags = []


if category_name:
category = Category(category_name)
else:
Expand All @@ -477,7 +499,7 @@ def comma_split(string):
category, description = None, None

activity = Activity(activity_name, category=category)
return cls(activity, start, end=end, description=description)
return cls(activity, start, end=end, description=description, tags = tags)
Copy link
Collaborator

Choose a reason for hiding this comment

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

there should not be kwarg assignment should not be surrounded by whitespace as per PEP8


@property
def start(self):
Expand Down