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 date-time format, and adds decode_datetime #66

Merged
merged 1 commit into from
Oct 31, 2022

Conversation

nevans
Copy link
Collaborator

@nevans nevans commented Oct 14, 2022

Fixes date-time format, and adds decode_datetime.

Also:

  • adds "date" gem as a dependency
  • adds Date handling to send_data. This will be used by search.
  • reduce duplication by using encode_datetime in send_time_data.

I don't know if anyone is actually using Net::IMAP.format_datetime. If they are, then fixing the bug will not be backwards compatible. Since "encode" and "decode" are a natural pair (and we already had "encode_utf7"), I added new methods with those names and added deprecation warnings to the old methods.

@nevans
Copy link
Collaborator Author

nevans commented Oct 14, 2022

@hsbt @shugo My primary concern here is adding the "date" gem as a dependency. I think it's the right thing to do. It's been a default gem since 2.5 and is still compatible down to 2.4. So there really shouldn't be any problems with it.

...but...

maybe there is?

@nevans nevans requested review from hsbt and shugo October 14, 2022 17:55
@nevans
Copy link
Collaborator Author

nevans commented Oct 15, 2022

Looking at https://rubygems.org/gems/date/reverse_dependencies, it looks like there aren't any very large/popular gems with a dependency on date. This would pull date into the rails dependency network. I'm of the opinion that explicit stdlib gem dependencies are good, but they come with "growing pains" that we eventually need to just push through and fix.

@hsbt, @shugo, what do you think?

`Net::IMAP.format_datetime` was previously using an incorrect
`date-time` format (see the `Net::IMAP::STRFDATE` rdoc).  I don't know
if anyone actually *uses* `format_datetime`, but fixing the bug isn't
backwards compatible.  I implemented the correct behavior under a
different method name (and an alias).  The existing method keeps its
original behavior (for now), with a warning that it will be fixed in a
future version.

Also:
* adds "date" gem as a dependency
* adds `{format,parse}_date` methods.
* updates `send_data`
  * adds `Date` handling.  This can be used by `SEARCH`.
  * uses `encode_datetime` from `send_time_data` (reduces duplication)
* adds aliases
  * `format_*`    => `encode_*`
  * `parse_*`     => `decode_*`
  * `encode_time` => `encode_datetime`
  * but `decode_time` returns a Time object
Copy link
Member

@shugo shugo left a comment

Choose a reason for hiding this comment

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

It's OK to add date.gem as a dependency.

@nevans nevans merged commit bda56f5 into master Oct 31, 2022
@nevans nevans deleted the bugfix-encode_datetime branch October 31, 2022 18:18
@mperham
Copy link

mperham commented Jan 5, 2023

There is a major issue with date: it requires native compilation. My server does not have a compiler installed for security reasons and I cannot install the latest version of net-imap now.

I install Ruby from distro packages and then a set of pure Ruby gems but net-imap is no longer pure. ☹️

@nevans nevans added the IMAP4rev1 Requirement for IMAP4rev1, RFC3501 label Feb 12, 2023
nevans added a commit to nevans/net-imap that referenced this pull request Nov 6, 2023
This modifies the code I added in ruby#66 to make it more useful when
`date-time` is parsed as a `quoted` string (as it currently is).  I'd
prefer that `date-time` values were simply parsed as DateTime objects in
the ResponseParser, but that has backwards compatibility issues.
nevans added a commit to nevans/net-imap that referenced this pull request Nov 6, 2023
This modifies the code added in ruby#66 to make it more useful when
`date-time` is parsed as a `quoted` string (as it currently is).  I'd
prefer that ResponseParser simply parse `date-time` values as DateTime
objects, but that has backwards compatibility issues.
nevans added a commit to nevans/net-imap that referenced this pull request Nov 6, 2023
This modifies the code added in ruby#66 to make it more useful when
`date-time` is parsed as a `quoted` string (as it currently is).  I'd
prefer that ResponseParser simply parse `date-time` values as DateTime
objects, but that has backwards compatibility issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IMAP4rev1 Requirement for IMAP4rev1, RFC3501
Development

Successfully merging this pull request may close these issues.

None yet

3 participants