Fixed the meridiem value for Chinese. #222

Closed
wants to merge 2 commits into
from

Projects

None yet

3 participants

@rockymeza

In Chinese, there are more values for the meridiem value than just AM and PM. There are values that mean morning, before noon, noon, afternoon, and night time. I had to modify moment.js to handle a callback or default to the previous behavior.

If you accept #221, I am pretty sure that you will have a merge conflict in the calendar day tests. Here is what those tests should look like:

// zh-cn.js
test.equal(moment(a).calendar(),                     "今天早上2点00",     "today at the same time");
test.equal(moment(a).add({ m: 25 }).calendar(),      "今天早上2点25",     "Now plus 25 min");
test.equal(moment(a).add({ h: 1 }).calendar(),       "今天早上3点00",     "Now plus 1 hour");
test.equal(moment(a).add({ d: 1 }).calendar(),       "明天早上2点00",     "tomorrow at the same time");
test.equal(moment(a).subtract({ h: 1 }).calendar(),  "今天早上1点00",     "Now minus 1 hour");
test.equal(moment(a).subtract({ d: 1 }).calendar(),  "昨天早上2点00",     "yesterday at the same time");
// zh-tw.js
test.equal(moment(a).calendar(),                     "今天早上2點00",     "today at the same time");
test.equal(moment(a).add({ m: 25 }).calendar(),      "今天早上2點25",     "Now plus 25 min");
test.equal(moment(a).add({ h: 1 }).calendar(),       "今天早上3點00",     "Now plus 1 hour");
test.equal(moment(a).add({ d: 1 }).calendar(),       "明天早上2點00",     "tomorrow at the same time");
test.equal(moment(a).subtract({ h: 1 }).calendar(),  "今天早上1點00",     "Now minus 1 hour");
test.equal(moment(a).subtract({ d: 1 }).calendar(),  "昨天早上2點00",     "yesterday at the same time");
Rocky Meza Fixed the meridiem value for Chinese.
In Chinese, there are more values for the meridiem value than just AM
and PM.  There are values that mean morning, before noon, noon,
afternoon, and night time.  I had to modify moment.js to handle a
callback or default to the previous behavior.
17aaf58
@timrwood
Moment.js member

Hmm, if we're adding moment.meridiem.lower and moment.meridiem.upper, should we just deprecate moment.meridiem.am/AM/pm/PM and default to hard coded am/pm?

I dont think any other languages use moment.meridiem.am besides south-east asian languages.

@rockymeza

Are you saying that in all of the other language files, am/AM/pm/PM are all the same?

@timrwood
Moment.js member

Yes, am/pm are the same for all languages but Korean and Chinese.

Meridiem was added for the Korean translation, and I didn't know if any future language would need different strings for upper and lower case, hence the semi-redundant am/AM, pm/PM.

@rockymeza

Is it possible to unify the meridiem method into one method? There is no point in having two methods if the only languages that would even use it don't even have uppercase/lowercase.

We could delete meridiem am/AM/pm/PM in all other language files and define a meridiem function that accepts hour, minute, case in those few languages that are different. That would shrink the filesize of most of the language definitions a little bit.

@timrwood
Moment.js member

Yeah, that's probably even better.

@rockymeza

This is the line that I am talking about in #222

@rockymeza

I went ahead and did that, but you don't have to merge that part of the pull request in if you don't want to.

I had to change something in the way that languages are added to moment that perhaps changed some functionality. I don't know if it is documented or intentional, but there is the capability for a language definition to inherit from another language definition. Previously, the language definition would inherit from the last language defined. Now it inherits from English. I don't know if this functionality was intentional. When I removed all of the meridiem references it broke the tests with regard to the languages after Korean.

Since English is default and is always available I decided that language definition files could inherit from it instead of disabling inheritance altogether.

@rockymeza

The tests pass if we take off inheritance or if we use the English as a fallback. However, I don't know about people's custom language definition files, if indeed there are any.

Thoughts?

@timrwood
Moment.js member

Yeah, there's no way to know about people's own language files, but I think we can document the upgrade path and what was depreciated from 1.5 to 1.6.

I think inheriting from english is a good idea. It gives the idea of a solid base, unlike how it currently acts, where it depends on the previous language. It will certainly shrink down this file too.

https://github.com/timrwood/moment/blob/master/lang/en-gb.js

@timrwood timrwood added a commit that referenced this pull request Mar 23, 2012
@timrwood timrwood remove am/AM/pm/PM from meridiem
Changing to a function.
#222 #228
b34ba94
@timrwood
Moment.js member

The pull request got botched, so I updated everything manually. Can you double check that the Chinese calendar day tests are correct?

b34ba94#L20R150
b34ba94#L21R150

@rockymeza
@timrwood
Moment.js member

Yeah, they pass, but I kinda went about it backwards, copy/pasting the results of the test back into the unit tests themselves. If it all looks good to you, I'll close this out.

@timrwood timrwood closed this Mar 26, 2012
@rockymeza

fine by me

@rockymeza rockymeza referenced this pull request Apr 8, 2012
Closed

Release 1.6.0 discussion #268

@jimmyye

There is a bug in the meridiem function.

            meridiem : function (hour, minute, isLower) {
                if (hour < 9) {
                    return "早上";
                } else if (hour < 11 && minute < 30) {
                    return "上午";
                } else if (hour < 13 && minute < 30) {
                    return "中午";
                } else if (hour < 18) {
                    return "下午";
                } else {
                    return "晚上";
                }
            },

When 11:31 am, it should be "中午" (noon), but it becomes "下午" (afternoon).
&& minute < 30 is wrong and unnecessary because noon means 11:00 to 13:00 in China.
This should work:

            meridiem : function (hour, minute, isLower) {
                if (hour < 9) {
                    return "早上";
                } else if (hour < 11) {
                    return "上午";
                } else if (hour < 13) {
                    return "中午";
                } else if (hour < 18) {
                    return "下午";
                } else {
                    return "晚上";
                }
            },
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment