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

Fixes Issue #662 - Day names not treated consistently for new entry #665

Merged
merged 4 commits into from
Oct 25, 2019

Conversation

notbalanced
Copy link
Contributor

When writing tests for this I also found a missing fix for issue #283 that somehow wasn't in the time.py anymore (maybe 2.0)?.

Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

This looks great. Just have one question about some old come coming back up.

Comment on lines +9 to +20
try: import parsedatetime.parsedatetime_consts as pdt
except ImportError: import parsedatetime as pdt
import time
import os
import json
import yaml
import keyring

consts = pdt.Constants(usePyICU=False)
consts.DOWParseStyle = -1 # Prefers past weekdays
CALENDAR = pdt.Calendar(consts)

Copy link
Member

Choose a reason for hiding this comment

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

Did we mean to put this back? This was removed in earlier work. Is there a reason this needs to go back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I was using a phrase in core.py check_output_time_inline for a couple of my scenarios testing the -on command line option on a certain date. When I searched the code, it wasn't being used anywhere.

Doing a little digging, it had been used in the dayone.feature file for testing entries that didn't have timezone information (i think). However all those tests have been removed.

The function didn't work for my needs because it didn't handle day names (i.e. 'monday', etc.) So I called the parsedatetime library instead of the date parser.

That is why I put the code in. All the tests passed, so I didn't look any further. :-)

Comment on lines -183 to +192
utc_time = date_parser.parse(text)
local_date = utc_time.astimezone(local_tz).strftime("%Y-%m-%d %H:%M")
assert local_date in out, local_date
date, flag = CALENDAR.parse(text)
output_date = time.strftime("%Y-%m-%d %H:%M",date)
assert output_date in out, output_date
Copy link
Member

Choose a reason for hiding this comment

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

Did we mean to put this back? This was removed in earlier work. Is there a reason this needs to go back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above comment.

Comment on lines +56 to +59
date = datetime(*date[:3],
hour=23 if inclusive else default_hour or 0,
minute=59 if inclusive else default_minute or 0,
second=59 if inclusive else 0)
Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

if len(date_str) <= 6:
# Don't try to parse anything with 6 or less characters and was parsed from the existing journal.
# It's probably a markdown footnote
if len(date_str) <= 6 and bracketed:
Copy link
Member

Choose a reason for hiding this comment

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

💯

@wren wren added the bug Something isn't working label Oct 5, 2019
@wren
Copy link
Member

wren commented Oct 15, 2019

@notbalanced Just letting you know that I didn't forget about this. Had a lot of life stuff come up the past few weeks, but I'll be working on jrnl this weekend, and will prioritize getting this properly reviewed. Thanks for your patience!

@wren wren changed the base branch from master to v2.1.2 October 24, 2019 23:59
@wren wren added this to the v2.1.2 - Non-critical bug fixes milestone Oct 25, 2019
Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

@notbalanced Thanks, again! Both for the PR and for your patience with the review process.

@wren wren merged commit 7511318 into jrnl-org:v2.1.2 Oct 25, 2019
wren added a commit that referenced this pull request Nov 26, 2019
commit 7511318
Merge: 74d1854 47e10fb
Author: Jonathan Wren <9453067+wren@users.noreply.github.com>
Date:   Thu Oct 24 17:02:10 2019 -0700

    Merge pull request #665 from notbalanced/issue_662

    Fixes Issue #662 - Day names not treated consistently for new entry

commit 74d1854
Merge: 1bbf074 6a5726a
Author: Jonathan Wren <9453067+wren@users.noreply.github.com>
Date:   Sat Oct 5 15:30:57 2019 -0700

    Merge pull request #418 from philipsd6/2.0-fancy_exporter

    Add exporter to output entries inside unicode box character boxes

commit 47e10fb
Author: Craig Moyer <craig.moyer@gmail.com>
Date:   Sun Sep 29 19:06:53 2019 -0400

    Fix issue #662 to properly handle day names as new entry dates and
    command line (-on, -from, -to).

commit 9588913
Author: Craig Moyer <craig.moyer@gmail.com>
Date:   Sun Sep 29 08:27:27 2019 -0400

    Syncing with jrnl-org/master

commit 4c68eb1
Merge: 81dfebb 1bbf074
Author: Craig Moyer <craig.moyer@gmail.com>
Date:   Sun Sep 29 07:52:02 2019 -0400

    Merge remote-tracking branch 'upstream/master' into 2.0-rc1-maebert

commit 81dfebb
Author: Manuel Ebert <manuel@1450.me>
Date:   Mon Apr 29 20:34:18 2019 +0200

    export changes

commit 6a5726a
Author: Philip Douglass <philip@philipdouglass.com>
Date:   Fri Dec 22 20:56:36 2017 -0500

    Enable FancyExporter plugin

commit 3d1b226
Author: Philip Douglass <philip@philipdouglass.com>
Date:   Fri Jan 29 11:17:41 2016 -0500

    Add exporter to output entries inside unicode box character boxes
@notbalanced notbalanced deleted the issue_662 branch March 8, 2020 18:53
wren added a commit that referenced this pull request Apr 18, 2020
commit 7511318
Merge: 74d1854 47e10fb
Author: Jonathan Wren <9453067+wren@users.noreply.github.com>
Date:   Thu Oct 24 17:02:10 2019 -0700

    Merge pull request #665 from notbalanced/issue_662

    Fixes Issue #662 - Day names not treated consistently for new entry

commit 74d1854
Merge: 97e4d6a 6a5726a
Author: Jonathan Wren <9453067+wren@users.noreply.github.com>
Date:   Sat Oct 5 15:30:57 2019 -0700

    Merge pull request #418 from philipsd6/2.0-fancy_exporter

    Add exporter to output entries inside unicode box character boxes

commit 47e10fb
Author: Craig Moyer <craig.moyer@gmail.com>
Date:   Sun Sep 29 19:06:53 2019 -0400

    Fix issue #662 to properly handle day names as new entry dates and
    command line (-on, -from, -to).

commit 9588913
Author: Craig Moyer <craig.moyer@gmail.com>
Date:   Sun Sep 29 08:27:27 2019 -0400

    Syncing with jrnl-org/master

commit 4c68eb1
Merge: 81dfebb 97e4d6a
Author: Craig Moyer <craig.moyer@gmail.com>
Date:   Sun Sep 29 07:52:02 2019 -0400

    Merge remote-tracking branch 'upstream/master' into 2.0-rc1-maebert

commit 81dfebb
Author: Manuel Ebert <manuel@1450.me>
Date:   Mon Apr 29 20:34:18 2019 +0200

    export changes

commit 6a5726a
Author: Philip Douglass <philip@philipdouglass.com>
Date:   Fri Dec 22 20:56:36 2017 -0500

    Enable FancyExporter plugin

commit 3d1b226
Author: Philip Douglass <philip@philipdouglass.com>
Date:   Fri Jan 29 11:17:41 2016 -0500

    Add exporter to output entries inside unicode box character boxes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants