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

Cell::Time downcase format before formatting #409

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pacoguzman
Copy link

In the case the format is on upcase we must downcase before trying to get convert to a strftime format expression. Is the same the library is doing on DateTime cells here

@coveralls
Copy link

coveralls commented Jan 3, 2018

Coverage Status

Coverage remained the same at 92.404% when pulling 2ccb6f5 on bebanjo:time-formatting into 62b033c on roo-rb:master.

@stevendaniels
Copy link
Contributor

@pacoguzman - can you provide an example of an actual spreadsheet that uses this format?

@tgturner
Copy link
Contributor

@pacoguzman I think the downcase is actually unnecessary here. I believe that we could just do something like this:

diff --git a/lib/roo/excelx/format.rb b/lib/roo/excelx/format.rb
index 1c1968a..52d70fa 100644
--- a/lib/roo/excelx/format.rb
+++ b/lib/roo/excelx/format.rb
@@ -5,8 +5,8 @@ module Roo
     module Format
       extend self
       EXCEPTIONAL_FORMATS = {
-        'h:mm am/pm' => :date,
-        'h:mm:ss am/pm' => :date
+        'h:mm AM/PM' => :date,
+        'h:mm:ss AM/PM' => :date
       }

and we could avoid it all together. Is this something you would be interested in taking on?

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.

4 participants