Add singular/plural forms for durations and startOf/endOf? #296

Closed
timrwood opened this Issue May 1, 2012 · 6 comments

Projects

None yet

2 participants

@timrwood
Member
timrwood commented May 1, 2012

See this comment

Adding support for either form adds 136 bytes to the minified version and 39 bytes to the gzipped version.

@rockymeza
Contributor

I don't like this idea. More ways than one to do the same thing seems like a bad idea. Also, I'm not a fan of putting more things on the moment global namespace. If we do put constants on the moment global, it should be under a namespace like moment.constants, but I don't like the idea of constants anyway.

I know that this is JavaScript, but I think that The Zen of Python speaks to this well:

There should be one-- and preferably only one --obvious way to do it.
Although that way may not be obvious at first unless you're Dutch.

@timrwood
Member

The counter argument would be that moment.startOf('days') would fail silently when it should be moment.startOf('day').

I'm not sure how much affordance should be given to ease developer headaches, but I'm still leaning towards supporting plural/singular, at least in startOf/endOf. We can just pull off the last 's' at the beginning of the function, something like this...

startOf: function (val) {
    var output = this.clone();
    val = val.replace(/s$/, ''); // remove the last s to support both plural and singular
    switch (val) {
        // cases here
    return output;
}
@rockymeza
Contributor

Alternatively, we could provide code that throws an error when they use the wrong form.

I don't feel super strong about either direction, I just want to make sure that we do consider both sides. Here is another tidbit from The Zen of Python:

Errors should never pass silently.
Unless explicitly silenced.

As long as we are not using constants, I am happy with whichever solution.

@timrwood
Member

Yeah, I think constants are a bit of overkill.

I'm hesitant to throw errors because it halts execution on IE and your whole site is broken after the thrown error. Also, it would be about the same amount of code as adding the plurals as outlined above, so there's not much benefit I think.

@timrwood
Member

I think the best solution would be to change this line to the following.

switch (val.replace(/s$/, '')) { // strip off the last s
@timrwood timrwood added a commit that referenced this issue Jun 11, 2012
@timrwood timrwood Whitespace diff… 19e15d6
@timrwood
Member

This is done and will go out in 1.7.0

@timrwood timrwood closed this Jun 11, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment