[WIP] Message structure refactoring

[svallory]

Unfortunately I couldn't finish it today. But enough waiting! Here's a preview of what is going on so you guys can peer review and help me shape it in the best way possible. (original issue: #286)

[svallory]

you can run DEBUG=1 node ./test-messages.js to checkout the parsing process and the result.

[keithamus]

Some excellent work @svallory. Really good. Quite a lot to digest though - so I'll make some general notes about what I see and we can discuss them further:

Syntax

I want to try and avoid too much bikeshedding about syntax but to me it feels like the biggest benefit of the new [] syntax is flag switches - but it has too much dressing around it.

• The : in [?:] is redundant when it already has ?.
• [+:], [@:] [-:] [#:] all add specific logic which adds extra complexity; harder to learn, harder to read, harder for us to maintain.
• [@:{{act}}] syntax feels like too much magic, and is too syntax dense. It doesn't feel like something that is repeated so much that it needs a shorthand syntax.

The way I see it - all of these could be normalised by using ternary operations based on flags(). The [] syntax gets normalised to: [<flag>?<flag-is-true-message>:<flag-is-false-message>] which simplifies parsing and makes it easier for implementors to digest. You're basically there with the existing flag code ([<flag>?:<flag-is-true-message>|<flag-is-false-message>]):

• [contains?:contain|have] becomes [contains?contain:have] (slightly less syntax, syntax is more familiar to JS devs).
• [-:not ] becomes [negate?:not ] (slightly longer - but less effort for us tokenising the -/+ tokens).
• [+:at least][-:below] becomes [negate?below:at least].
• [+:true][-:false] becomes [negate?false:true]
• [#nkeys:key|keys] becomes [nkeys?key:keys] (we'd have to change the nkeys flag value to a boolean though)
• [@:{{act}}] would become something like [an?an:a] {{act}} - but you'd have to add flag(this, 'an').

Minor Issues

• You've sometimes got the message listed twice, and others you've put null, /* deprecated */ - this should ideally be consistent between every message.
• The SOMETHING constant threw me a bit. Maybe a premature optimisation. Feels like its sacrificing code readability.
• I don't want to turn this into a bikeshedding exercise (I probably should have spoken up in the issue) - but the message syntax felt a little cryptic to me on first pass:
• The change from #{} to {{}} syntax makes this PR a breaking change, rather than a nice extension on existing messages (because of our plugin ecosystem). It would be excellent if we could avoid this.
• You've refactored getMessage to just drop the negated message - but could retain it for backwards compat.
• The failing tests are important to fix, most are regressions in functionality (specifically the to.throw() syntax seems to have regressed, and some not equals are coming up as to equal which is obviously wrong).

Plus points

• Obviously the message framework as a whole is a huge plus
• The meta messages is an excellent addition. I'm sure this will be useful for plugin authors
• The extra wording on some of the assertions is a welcome addition (e.g. ...to have 'length' with value 4, but got 3)

Feel free to challenge/disagree with any of my points. These is mostly me thinking out loud while looking through the code - and so every point could be discussed further.

[logicalparadox]

In addition to @keithamus comments, the new constructors that have been added should have each method commented according to the our style guidelines. It just makes things a lot easier to maintain, especially with multi-author projects like this. While this should not be a high priority and is probably best done close to last, it is a requirement for merging.

Overall, this is exciting stuff! Looking forward to seeing how this goes.

[svallory]

A lot of valid points @keithamus! Before we discuss of them know that the code isn't uniforme yet because it's a WIP, hence the double messages vs null /* deprecated */, lack of docs and failing tests (as I've warned on the issue). It will all be fixed before merging, so don't worry about it.

Syntax

Here's all you need to know about tokens:

{{var}} - will print the value of a variable
[+:text] - will print "text" if the message isn't negated
[-:text] - will print "text" if the message IS negated
[var?:true|false] - Will print "true" if var is truthy and "false" if it's falsy
[#var:zero|one|many] - Will select a word according to the value of var
[@:{{var}}] - will add a proper "a" or "an" prefix to the value of var

That's all there's to know and I don't think it's hard. Of course, it's a lot more to maintain. But each piece is simple and don't think there will be much maintaining to do.

Also, these tokens don't add extra logic to chai (except the switch token). All of this was already supported, the tokens just move the logic for the proper place, the message.

My points on why not use only the switch token

• The switch wouldn't support 3 cases like [#] does
• If we drop the [@] we have to put the logic for selecting a/an elsewhere and call it when we are preparing a value for the message. Also, a message can have more than one [@] place and one flag wouldn't cut it.
• Having [+] and [-]doesn't stop us from writing [negate?false:true] but makes it easier when we want to write `'...to [-:not ]have...'

About the extra dressing

I tried to make it as little as possible while keeping it uniform. The initial design for a token was [(TOKEN)(variable):(CONTENT)] this actually makes parsing easier. Where:

• TOKEN Is one of (+, -, #, @, ?)
• variable Is "optional" in the sense that it's token dependant
• : The colon is a token/content separator.
• CONTENT Is passed to the token so it can work it's magic

But [var?:yes|no] looked better than [?var:yes|no] so I moved it. But maybe I should put it back at the beginning.

We could drop the : by moving the symbol to it's place. The new syntax would look like this:

"Expected {{this}} [-not ]to [contains?contain|have] [nkeys#no keys|{{nkeys}} key|{{nkeys}} keys] but it does[+n't]. It's [@{{type}}]"

But I think this is easier to read, specially over time, because you get used to look for the : and your brain automatically splits the info.

"Expected {{this}} [-:not ]to [contains?:contain|have] [nkeys#:no keys|{{nkeys}} key|{{nkeys}} keys] but it does[+:n't]. It's [@:{{type}}]"

#{this} vs {{this}}

We can keep the #{} syntax, but I'm using the # for the pluralize token (it's the best symbol for it), so I would prefer to change it to {{}}. We could keep the support for the old syntax while advertising the new one. Once plugins have moved to the new syntax (and there's no rush on that) we remove it. How does that sound?

About SOMETHING and concepts

The SOMETHING constant wasn't actually an optimization. I left it there because I was focused on something else but I knew I would have to come back to this.

There's a difference between an expected value and it's representation on the message. I think expected should be inspectable and meaningful. If we simply set expected to "something", how would the user be able to differentiate the concept from the string? He wouldn't. So what we want is to create a structure which will represent the concept to set it apart from a simple string.

Other concepts I can think of are: nothing, truthy and falsy but there may be more. Also concept classes could have a method like isSatisfiedBy(obj) which returns a boolean indicating whether an object falls within the concept boundaries.

My questions now are: Should we implement this? Alongside the meta classes? If not, where?

Notes

• I've change the getMessage method to use the negate message using msg = (negate ? args[2] : args[1]) || args[1]
• I'm thinking about supporting deep properties on ? and #. It would allow us to write [#keys.length:key|keys] and drop the flag.
• I would like some help on naming. I hate 'PseudoNode' but I couldn't find a better name.
• Some opinions on how to structure chai.messages would be good too. Currently both messages and meta types are on the same "namespace" (eg: chai.messages.lengthOf and chai.messages.Type). Should I use chai.messages.meta.Type? chai.meta.Type? What about the Message class? Where should it go?

[keithamus]

Almost all of the syntaxes you add are effectively ternary operations (they assert on value, and depending on truthiness they pick which expression to follow).

You've put a bit more complexity in the PluralizeNode - but I think its still lacking. For starters - it only supports 3 switches (0, 1 or more than one). This limits flexibility and, if we were to go down an i18n route - would not be compatible with the myriad of plural forms across various locales (http://unicode.org/repos/cldr-tmp/trunk/diff/supplemental/language_plural_rules.html). You said ternaries could not solve this - but I think they can... bear with me here - and this is only a jumping off point, I'm not prescribing this, just putting it here to discuss further.

Adopt Ternary syntax

If we extend our existing string interpolation - so we have #{var} which can be used to drop in a var (like we have right now), and #{op?choice-a:choice-b} which is our ternary switch. Wether the interpolation is a var or ternary is determined by the ?; e.g.

function getInterpolatedParts(string) {
    var match = string.match(/#\{([^\?]+)(\?([^:]+)(?:\:(.*))?)?\}/);
    return {
        property: match[1],
        isTernary: match[2] !== undefined,
        truthy: match[3] || '',
        falsey: match[4] || '',
    };
}
getInterpolatedParts('#{foo}').should.eql({ property: 'foo', isTernary: false, truthy: '', falsey: '' });
getInterpolatedParts('#{foo?bar}').should.eql({ property: 'foo', isTernary: true, truthy: 'bar', falsey: '' });
getInterpolatedParts('#{foo?bar:baz}').should.eql({ property: 'foo', isTernary: true, truthy: 'bar', falsey: 'baz' });

Properties support deep notation

We already have code available for parsing deep notation - we can leverage this to enable deep property access within our interpolated strings, meaning #{keys.length} would work - as would #{keys.length?none:some}

Properties support assertions

We already have code that supports asserting against operators - this could be pulled out into a generic util lib that could be used within chai messages, this way we could do more advanced operations within our interpolated sections, such as #{keys.length===0?no keys:some keys}, but also give developers more flexibility in their messages for things like #{type=='object'?an :a}

Support nested interpolations

We could support nested interpolations, which could help two fold: one being that we could compose messages out of smaller messages - which themselves could be interpolated, the other being that we could have nested ternaries - which would solve the pluralisation problem much more effectively. See the following two, which support these:

Composition:

utilMessages = { toBeOrNotToBe: '#{negate?to be:to not be}' };
message = 'expected #{this} #{utilMessages.toBeOrNotToBe} a #{type}';
// expected foo to be a bar
// expected foo to not be a bar

Nested Ternary:

message = 'expected #{this} to have #{keys.length==0?no:#{keys.length==1?one:many}} #{keys.length==1?key:keys}'
// expected foo to have no keys
// expected foo to have one key
// expected foo to have many keys

You could even end up going really crazy with nested ternaries to support very fine grained pluralisation (line breaks & tabs added for readability):

expected #{this} to#{negate?: not} #{contains?contain:have}
#{keys==0?
    no keys:
    #{keys==1?
        one key:
        #{keys==2?
            two keys:
            #{keys<6?
                a few keys:
                many keys
            }
        }
    }
} of #{exp}

Thoughts @logicalparadox & @svallory?

[keithamus]

Hey @svallory thanks for all of your hard work on this, it has really helped us with where we're headed with chai and your work was amazing. I think I'm going to close this for now though, as it hasn't had much activity recently, and if we go in the direction that #585 is generally pointing in, we may have interesting ways to auto generate messages from assertions.

We may come back to this at some point in the future, if and when we do I'll re-open this and let you know @svallory 😄.