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

Remind contexts and apply to time refs #82

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

YuriRomanowski
Copy link
Contributor

@YuriRomanowski YuriRomanowski commented Feb 24, 2023

Description

Problem: Sometimes, context of a time reference can be spread over one sentence, several different sentences or even several different messages.

Solution: Parse contexts alone, remind them and apply to context-free time references when encountered; track a context during thread evolution.

Related issue(s)

Part of #16

✅ Checklist for your Pull Request

Related changes (conditional)

  • Tests

    • If I added new functionality, I added tests covering it.
    • If I fixed a bug, I added a regression test to prevent the bug from
      silently reappearing again.
  • Documentation

    • I checked whether I should update the docs and did so if necessary:

Stylistic guide (mandatory)

YuriRomanowski added a commit that referenced this pull request Feb 24, 2023
Problem: Sometimes, context of a time reference can be spread over one
sentence, several different sentences or even several different messages.

Solution: Parse contexts alone, remind them and apply to context-free time
references when encountered; track a context during thread evolution.
@YuriRomanowski YuriRomanowski force-pushed the YuriRomanowski/remind-contexts-and-apply-to-time-refs branch from b390131 to ec5c7ca Compare February 24, 2023 13:14
@YuriRomanowski YuriRomanowski force-pushed the YuriRomanowski/#79-add-support-for-hyphenated-intervals branch 2 times, most recently from 2dcf198 to 461920b Compare February 24, 2023 20:04
Problem: when a user writes "10am-11am UTC ...", context is not shared
between times, and second time can be skipped because every time should
go after a space.

Solution: Parse time references that are grouped via hyphen, slash,
"and", "or" sharing their context: date ref, location ref, am/pm.
@YuriRomanowski YuriRomanowski force-pushed the YuriRomanowski/#79-add-support-for-hyphenated-intervals branch from 461920b to 148360b Compare February 24, 2023 20:25
YuriRomanowski added a commit that referenced this pull request Feb 24, 2023
Problem: Sometimes, context of a time reference can be spread over one
sentence, several different sentences or even several different messages.

Solution: Parse contexts alone, remind them and apply to context-free time
references when encountered; track a context during thread evolution.
@YuriRomanowski YuriRomanowski force-pushed the YuriRomanowski/remind-contexts-and-apply-to-time-refs branch from ec5c7ca to 9d9bfde Compare February 24, 2023 21:11
@YuriRomanowski YuriRomanowski marked this pull request as ready for review February 24, 2023 21:11
YuriRomanowski added a commit that referenced this pull request Feb 24, 2023
Problem: Sometimes, context of a time reference can be spread over one
sentence, several different sentences or even several different messages.

Solution: Parse contexts alone, remind them and apply to context-free time
references when encountered; track a context during thread evolution.
@YuriRomanowski YuriRomanowski force-pushed the YuriRomanowski/remind-contexts-and-apply-to-time-refs branch from 9d9bfde to e32b0bf Compare February 24, 2023 21:14
@YuriRomanowski
Copy link
Contributor Author

YuriRomanowski commented Feb 24, 2023

Looks much better 😅
изображение

@@ -92,8 +92,12 @@ run opts = do
managed $ withTzCacheDefault defaultMessageInfoCachingTime
bsReportEntries <-
managed $ withTzCacheDefault cCacheReportDialog
-- auto-acknowledge received messages

let defaultConversationStateCachingTime = hour 12
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This ideally should be in the config

Cache.insert getMessageThreadId (msgId, newState) convStateCache
asks bsMessageCache >>= Cache.insert msgId (timeRefs, conversationState)

withNonEmptyTimeRefs timeRefs $ \neTimeRefs -> do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that we will eventually rewrite this logic using some abstracting technique and write some tests, it gets a bit tricky as more internal state is kept.

Problem: Sometimes, context of a time reference can be spread over one
sentence, several different sentences or even several different messages.

Solution: Parse contexts alone, remind them and apply to context-free time
references when encountered; track a context during thread evolution.
@YuriRomanowski YuriRomanowski force-pushed the YuriRomanowski/remind-contexts-and-apply-to-time-refs branch from e32b0bf to aa1eb6f Compare February 24, 2023 21:31
@dcastro dcastro force-pushed the YuriRomanowski/#79-add-support-for-hyphenated-intervals branch 2 times, most recently from e3cc9f3 to fff943d Compare March 31, 2023 17:39
@dcastro dcastro force-pushed the YuriRomanowski/#79-add-support-for-hyphenated-intervals branch from 369fbef to 4756b9b Compare August 21, 2023 14:41
Base automatically changed from YuriRomanowski/#79-add-support-for-hyphenated-intervals to main August 21, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant