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.

Sign up for GitHub

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

Jump to bottom

Uncomment and fix filter tests #241

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Uncomment and fix filter tests #241

wants to merge 3 commits into from

Conversation

psethwick
Copy link

@psethwick psethwick commented Aug 17, 2023 •
edited
Loading

Fixes:

  1. test code incorrectly setting up Items with label ids instead of names (test-only fix)
  2. Item.DateTime() now checks Item.Due.TimeZone and uses it
    • root cause: daylight savings time (which JST does not have)
    • (in order for the tests to pass I also set them in the same timezone the tests expect)
  3. filter parser was getting stuck on times in pm because .Scan() tried to parse it as a float

ref #210

@psethwick
uncomment 2 filter tests + simplify Eval labels
ea6d5d8 
Eval does not need the labels collection. The label names already on the item are sufficient
@psethwick
apply item timezone and fix date filter tests
925adbd
@psethwick
filter parser can now parse afternoon times in am/pm format
bec57b5 
the default Scanner mode includes floats and .Scan() was getting 3p (test example) and fails to make an actual floating point number out of it
excluding scanner.ScanFloat avoids that possibility
@psethwick psethwick mentioned this pull request Sep 23, 2023
Open
@psethwick
Copy link
Author

This PR was really just preparing for what I wanted to do, which was more fully implement the filter language.

I have done the further work, you can see it here:
https://github.com/psethwick/todoist/pull/1/files

How should I proceed, should I put those in this PR? I know it is a lot of change :-/

Happy to rework things here and there, too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@psethwick