HalosGhost / pbpst

A command-line libcurl C client for pb deployments
GNU General Public License v2.0
44 stars 9 forks source link

Teach -x time unit shortcuts #74

Closed tmplt closed 7 years ago

tmplt commented 7 years ago

While calculating 60 * [minutes|60 * hours] isn't the most difficult of math problems, it still slows down the usage of the program. Could suffixes like -x 3h and -x 30min be implemented?

HalosGhost commented 7 years ago

Okay, here's where this gets dicey: how complex are you hoping the sufficies to be?

e.g., are you hoping to be able to say -x 3h22m12s, -x 3.75h or will allowing only whole numbers and a single suffix be sufficient?

If the restriction of a single suffix and an integer value is acceptable, then this does not bother me at all, and it should not take long to implement. If you are hoping for more complex values, this could be a rabbit-hole of parsing.

tmplt commented 7 years ago

A single suffix and integer is enough for my usages.

For more complex sunsets like -x 3.75h -x $(bc <<< "3.75 * 60 ^ 2" could be used — which does print it as a float; I'm unsure if pbpst truncates to int. If it doesn't, perhaps it could ignore the floating part (e.g. interpret 3.75 as 3) instead of having to create a trivial wrapper script?

HalosGhost commented 7 years ago

It should, at the moment, truncate (we use scanf()).

One other consideration: I suspect the sufficies I will support will be the following: d(ays), h(ours), m(inutes), s(econds).

However, all of the sufficies (other than seconds) would actually just be a shortcut for seconds (e.g., 1d == 86400s which is not, strictly speaking, true). Is that amenable? Dealing with time in an accurate fashion is incredibly difficult, so it should just be known that these units are not “correct”, they are just shorthand for seconds.

tmplt commented 7 years ago

I'm not entirely use what you mean. A day (and night) is 24 which is 24 * 60 ^ 2 == 86400s; is that not accurate?

HalosGhost commented 7 years ago

Not quite; things like leap seconds tend to throw a wrench in the mix.

For a list of things that devs often assume incorrectly about time, see this article.

tmplt commented 7 years ago

I see. Well, I'm okay with that caveat.

HalosGhost commented 7 years ago

All right; in that case, this should not be difficult to do.

HalosGhost commented 7 years ago

@Tmplt, a20ed59807d1df5288f1bae12701307a89e70135 adds this functionality.

It scans for a double, but casts to an int, so it will truncate the value; in addition, it only checks for a single character, which means anything following that is ignored.

$ pbpst -S#f somefile -x 5m

This means that the above will set a sunset 300 seconds into the future and is exactly the same as this:

$ pbpst -S#f somefile -x 5.7min

Finally, to remain backwards compatible, and to gracefully fallback to something sensible, if you pass an unexpected unit, or you pass no unit at all, pbpst assumes you meant seconds.

Can you test this and make sure it works the way you expect? I will then document it and close the issue.

tmplt commented 7 years ago

It works as expected. Thank you!

HalosGhost commented 7 years ago

Good to hear it. I am going to change a bit of the implementation to be safer (casting from a double to an int without a few checks is a BadIdea™), and simpler (deduplicating some code). But it should not change the behavior.

I will push the documentation and updates tonight.

HalosGhost commented 7 years ago

ad45d65da4f17cbb550a6c4e98be7ecd6df64f42 closes this issue.

@Tmplt, one extra thing to note; I changed the cast to unsigned to take place a moment later which means that instead of truncating the count before the multiplier, it will truncate afterwards.

i.e., 5.5m == 330 rather than 300. I would suspect this is actually preferred,