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

Allow abbreviated time units (e.g. 30s, 5m, 1h, 3d) #144

Open
hastebrot opened this issue Oct 5, 2016 · 5 comments
Open

Allow abbreviated time units (e.g. 30s, 5m, 1h, 3d) #144

hastebrot opened this issue Oct 5, 2016 · 5 comments

Comments

@hastebrot
Copy link

Examples:

$ t in -a 30s
$ t in -a "30s ago"
$ t edit -e "in 5m"
$ t edit -s "1h ago"

Prototype:

diff --git a/lib/timetrap/timer.rb b/lib/timetrap/timer.rb
index d0f3450..58ea676 100644
--- a/lib/timetrap/timer.rb
+++ b/lib/timetrap/timer.rb
@@ -16,7 +16,7 @@ module Timetrap
         time
       when String
         chronic = begin
-          time_closest_to_now_with_chronic(time, now)
+          time_closest_to_now_with_chronic(replace_time_units(time), now)
         rescue => e
           warn "#{e.class} in Chronic gem parsing time.  Falling back to Time.parse"
         end
@@ -31,6 +31,18 @@ module Timetrap
       end
     end

+    def replace_time_units(time)
+      pattern = /\b(\d+)([smhd])\b/
+      subst = {"s" => "sec", "m" => "min", "h" => "hour", "d" => "day"}
+      new_time = time.gsub(pattern) { |capture| 
+        match = Regexp.last_match
+        value, unit = match[1], match[2]
+        subst.each { |key, val| unit.gsub!(key, val) }
+        "#{value} #{unit}"
+      }
+      if pattern.match(time).to_s == time then new_time + " ago" else new_time end
+    end
+
     def time_closest_to_now_with_chronic(time, now)
       [
         Chronic.parse(time, :context => :past, :now => now),
@dashkb
Copy link
Contributor

dashkb commented Mar 4, 2017

I know this is super old, but it probably belongs as a feature request on chronic where it could potentially be useful to many more people than just those using Timetrap.

Edit: maybe not such a good suggestion as Chronic's master branch hasn't moved in years.

@hastebrot
Copy link
Author

@dashkb

maybe not such a good suggestion as Chronic's master branch hasn't moved in years.

Indeed, it wasn't updated for a while. Also this PR is very useful for timetrap users who want to get rid of additional letters and spaces when using the command line. This might not be an important use-case for chronic users.

@dashkb
Copy link
Contributor

dashkb commented Mar 4, 2017

This might not be an important use-case for chronic users.

Totally. It's just that right now this gem does zero time parsing, and maybe that's an intentional choice.

@samg
Copy link
Owner

samg commented Mar 6, 2017

I do have some hesitation about adding additional natural time parsing directly to the timetrap gem. In theory it could be a convenient addition but it does open the door for a lot of potentially confusing bugs or behaviors.

Another approach that occurred to me would be to add some kind of middleware layer to the CLI. This would be similar to the custom formatters (but on the input side) and give timetrap users a way to write code that would pre-process their command line input and allow extensibility like what's described in this issue.

I don't have time these days to add any significant new features, but if someone was interested in exploring that approach, I'd be interested in pull requests.

@dashkb
Copy link
Contributor

dashkb commented Mar 6, 2017

I don't have time these days

Where has it all gone? Mine used to be more abundant as well.

if someone was interested in exploring that approach, I'd be interested in pull requests.

I'm somewhat interested. Not promising anything...

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

No branches or pull requests

3 participants