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

Add relative time formatting #389

Closed

Conversation

mbirtwell
Copy link
Contributor

Hi,

I wrote a couple of functions for formatting some date related things based on the cldr data.
getDay just looks up the names of days in the cldr data. (and supports the different formats that cldr supports)
formatRelativeCount formats an amount of time in a relative way. e.g. if you ask for 3 days in the past it will return 3 days ago. It tries to use words like today and tomorrow if available.

Michael Birtwell added 4 commits February 11, 2015 12:23
It formats offsets of a particular unit of time
Now returns words like today, yesterday, etc. if appropriate
@rxaviers
Copy link
Member

@mbirtwell, thanks for your contribution.

I will review your code soon. But, quick comments.

  1. Could you read&sign our CLA?
  2. Could you rewrite your formatRelativeCount according to Support relative time #205 (see also https://github.com/jquery/globalize/tree/fix-205-format-duration)?

@mbirtwell
Copy link
Contributor Author

I've signed the CLA
I'll have a look at converting the work that I've done for formatRelativeCount in to formatDuration. It looks like #205 potentially covers a lot more functionality than I have attempted to implement. So I'll start by trying to re-organising what I've done so far as clean subset of the functionality and let you know how I get on.

@mbirtwell
Copy link
Contributor Author

Ok I think I've got my head around most of the conversation in #205 what I propose doing is closing this pull request and creating two new ones.

One for getDay because I think that's useful as it is. Although arguably could live in cldrjs.

One for a relativeDuration function (the name is flexible), which will have the same API as my formatRelativeCount function which I think is very similar to the value + unit low level api that @ichernev is proposing. A full implementation could then be built on top of this. Unfortunately I want this api not the one that takes dates, so I'm not going to implement that.

Let me know what you think?

@rxaviers
Copy link
Member

getDay just looks up the names of days in the cldr data

This method is very similar to .formatDate( <date>, "E" ). It's also very specific (you would end up having another method for Month names, quarter names, era names and so on). Also, it can be replaced by one line using cldrjs cldr.main([ "dates/calendars/gregorian/days/stand-alone", format, day ]);. So, I'd recommend one to either use .formatDate() or cldrjs directly on that case.

@rxaviers
Copy link
Member

About the relative time format function, on #205 we came up with an API that has Dates as parameters (which is summarized in the description of #205). You are suggesting it should take integer values. I think we can extend our original API (that takes Dates) to accept either a Date or an integer value.

I suggest you to use our existing branch as a baseline and continue implementing from there. You could either close this PR and create a new one, or update this PR. I'm fine with either options.

If you are interested, I can rebase our existing branch to the current master (in order to get that up-to-date first). How does that sound to you?

@mbirtwell
Copy link
Contributor Author

I had a go at merging master in your fix-205 branch earlier and got that to
the point of passing grunt. So I think I can continue from there.

I think there really is two functions here one that takes dates and on that
takes integer and units. I'd imagine the one that takes dates would be
implemented using the other but it seems to me trying to combine them makes
the interface a bit confusing.

On Thursday, 12 February 2015, Rafael Xavier de Souza <
[email protected]> wrote:

About the relative time format function, on #205
#205 we came up with an API
that has Dates as parameters (which is summarized in the description of
#205 #205). You are
suggesting it should take integer values. I think we can extend our
original API (that takes Dates) to accept either a Date or an integer value.

I suggest you to use our existing branch as a baseline and continue
implementing from there. You could either close this PR and create a new
one, or update this PR. I'm fine with either options.

If you are interested, I can rebase our existing branch to the current
master (in order to get that up-to-date first). How does that sound to you?


Reply to this email directly or view it on GitHub
#389 (comment).

@rxaviers rxaviers changed the title Add more date functions Add relative time formatting Mar 26, 2015
@rxaviers
Copy link
Member

Superseded by #395

@rxaviers rxaviers closed this Mar 26, 2015
ashensis pushed a commit to ashensis/globalize that referenced this pull request Mar 17, 2016
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.

2 participants