andrewmcveigh / cljs-time

A clj-time inspired date library for clojurescript.
342 stars 57 forks source link

Is it bug in parsing meridiem? #112

Closed VyacheslavMik closed 6 years ago

VyacheslavMik commented 6 years ago

Hello. Thanks for your library!

Currently, I can parse such format hh:mm A with this value 10:00 T. I think that you forgot default clause in cond in this function:

(defn parse-meridiem
  ([]
   (fn [s]
     (let [[[m n] s] (split-at 2 s)
           meridiem (str m n)
           [meridiem s] (cond (#{"am" "pm" "AM" "PM"} meridiem)
                              [meridiem s]
                              (#{\a \p} m)
                              [({\a "am" \p "pm"} m) (cons n s)]
                              (#{\A \P} m)
                              [({\A "am" \P "pm"} m) (cons n s)])]
       [[:meridiem (keyword meridiem)] (string/join s)]))))

May be it will be better:

(defn parse-meridiem
  ([]
   (fn [s]
     (let [[[m n] s] (split-at 2 s)
           meridiem (str m n)
           err #(ex-info
                 (str "Invalid meridiem format: " meridiem) {:type :parse-error})
           [meridiem s] (cond (#{"am" "pm" "AM" "PM"} meridiem)
                              [meridiem s]
                              (#{\a \p} m)
                              [({\a "am" \p "pm"} m) (cons n s)]
                              (#{\A \P} m)
                              [({\A "am" \P "pm"} m) (cons n s)]
                              :default
                              (throw (err)))]
       [[:meridiem (keyword meridiem)] (string/join s)]))))
VyacheslavMik commented 6 years ago

Also, with this format I can use such values 999T and it's parsed without errors.

andrewmcveigh commented 6 years ago

Thanks for reporting. I think you're analysis is correct, a failure to parse should throw an error.

VyacheslavMik commented 6 years ago

Hello, @andrewmcveigh. Thanks for your answer! Do you fix this or may be create pull request?

andrewmcveigh commented 6 years ago

A PR would be awesome 👍

VyacheslavMik commented 6 years ago

Hello, @andrewmcveigh. I'm fixed this issues. How quickly you can review pull request?

andrewmcveigh commented 6 years ago

Fixed with #113