Can I refactor chai messages?

[svallory]

As of now Chai error messages are embedded into code and don't follow a strict rule. Take, for example, the assertAbove method...

when flag doLength is true, Chai uses

 'expected #{this} to have a length above #{exp} but got #{act}'

otherwise it uses

'expected #{this} to be above ' + n

This makes it harder to improve messages by overwriting the getMessages method, which was my approach to modify the message before it was sent away.

My idea is to extract all messages to a module which would return a hash so messages would be referenced by a string id. Something like...

lib/chai/core/messages.js

module.exports = {
    above: "expected #{this} to be above #{exp}",
    aboveLength: "expected #{this} to have a length above #{exp} but got #{act}"
}

This would allow plugins to translate, improve, add color or even replace all messages to the liking of the author.

I'm volunteering to code it and make a pull request. Should I proceed?

[vesln]

That makes sense to me, @logicalparadox?

[svallory]

I also have a question. Why, in some cases, chai doesn't show the actual value? e.g.

  function an (type, msg) {
    if (msg) flag(this, 'message', msg);
    type = type.toLowerCase();
    var obj = flag(this, 'object')
      , article = ~[ 'a', 'e', 'i', 'o', 'u' ].indexOf(type.charAt(0)) ? 'an ' : 'a ';

    this.assert(
        type === _.type(obj)
      , 'expected #{this} to be ' + article + type
      , 'expected #{this} not to be ' + article + type
    );
  }

While refactoring messages I could also make sure expected and actual is always passed to assert(). In this case it would look like this:

this.assert(
        type === _.type(obj)
      , msg[article]
      , msg.not[article]
      , type
      , _.type(obj)
    );

Is there a reason for not doing this?

[logicalparadox]

Yeah, go for it. The reason this hasn't been done yet is probably because nobody offered to do it ;)

[svallory]

Hey guys, I'm almost done implementing the new messages structure by I'm facing a dilema: to have more, simpler messages or to have fewer, more complex messages. I started off with a structure like this:

{
  a: "expected #{this} to be a #{exp}",
  an: "expected #{this} to be an #{exp}",
  above: 'expected #{this} to be above #{exp}',
  . . .
  not: {
    a: "expected #{this} not to be a #{exp}",
    an: "expected #{this} not to be an #{exp}",
    above: 'expected #{this} to be at most #{exp}'
    . . .
  }
}

That's the simpler messages option. You'd use it like messages.a and messages.not.a

Then I though the messages had too little variation, specially between the normal and the negate messages of an assertion, so I came up with this:

{
  a: 'expected #{this} #{not}to be #{article:#{exp}}',
  above: 'expected #{this} to be #{+:above}#{-:at most} #{exp}',
}

And then I updated the getMessages method so...

• #{article:} automatically adds the appropriate article (a|an) for whatever comes later.
• #{-:} is only kept on the negate message
• #{+:} is only kept on the affirmative (I don't have a better name for it) message

This, in fact, eliminates the need for a negateMessage parameter. Then I thought I was going too deep and had to consult you guys. So what do you think?

[logicalparadox]

Very cool! I have a couple of thoughts before we get to removing negateMessage parameter.

• Could you change it so the template language differentiates between objects inserted (#{this} #{act} #{exp}) and language used (#[+:above], #[article:#{exp}], etc)?
• Maybe for article you could use something like @ or &. Your call for whatever symbol looks best.

Now, for negateMessage... You are welcome to change the core assertions to only have a single message but the function argument signature and implementation for Assertion#assert has to remain backward compatible. I do not want to "force" this template language onto plugin developers (yet).

My guess for the easiest way is that if the negation flag is present but no negation message was provided, assume that the normal message can be composed into a negated message.

Otherwise, looking good!

[svallory]

Good. Since I sent that message I went ahead with the "smarter" messages approach (it would be easy to revert anyway). I like the idea of differentiating placeholders from other template structures.

Here's a proposal based on what I felt is needed and your considerations:

• {{token}}: normal placeholders meant to be replaced by values
• [@:{{token}}]: same as above but adds an article (a or an) before the value.
• [#token: no keys|1 key|{{token}} keys]] or #{token:key|keys}#: selects the word based on the value of token. In the first case it takes no keys if token is 0, 1 key if it's 1 and {{token}} keys for every other case
• [deep?:deep prop|property]: returns 'deep prop' if flag(obj, 'deep') is truthy, otherwise, 'property'. Can also used as [?deep: deep] property
• [!deep?: first level]: does the inverse of the above
• [+: some message]: some message only remains on the normal message
• [-: some message]: some message only remains on the negate message
• I'll drop the #{not} in favor of [-:not]

Most of these is somewhat implemented (I'll have to update the syntax).
To fully support this I'll promote the messages to an immutable object and add a method which allows us to embed the values for the tokens in the message object itself. The reason for this is that I've noted sometimes what you want to print in the message as the 'expected' value is not the same of what we should set on the expected property of the assertion error. Most of the time it's a representation or a summary of it. But filling the placeholders in the assertions, like it was done before, prevent plugins from stepping because we loose the metadata.

In favor of metadata and to be able to control how data is displayed from a single place, I've created some "meta" classes in the namespace messages.meta, they are at the time of writing:

• Type: For type names
• List: For displaying arrays like ['to','from','when'] as 'to', 'from' and 'when'
• Range: For displaying ranges in the form 2..5
• RangeDelta: For displaying ranges in the form 2 +/- 1
• Unquoted: For preventing strings from being quoted by inspect

All this types implement the inspect and toString() methods so they are automatically converted either when appended to strings or console.log'ed.

Do you guys think there is any demand to translate chai messages? If so, I'll take that into account as I implement this.

[logicalparadox]

This all looks good to me; looking forward to seeing the code :)

I think that if this much work is being put into a new message manager, having multi-lingual support is a nice bit of icing on the cake.

@vesln Any thoughts?

[keithamus]

@svallory I'd love to see your progress on this. Would you like to put it in a PR so we can all look at it, and maybe continue on from your great sounding work 😄

[svallory]

Hey Guys, I'm sorry to leave this hanging for so long. I'll take a look at the code I've done, rebase it and make a PR as soon as possible.

[keithamus]

Hey @svallory, just checking in to see if you made any progress with this?

[svallory]

@keithamus and guys sorry for The reeeeealy huge delay on this. But I come bearing good news! I've got some time to work on it this week. I'll make a PR as soon as I get it to pass all the tests (hopefully today!). I have 18 to go.

[keithamus]

@svallory really looking forward to this. Can't wait 😄

[keithamus]

Closing this alongside #393:

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 😄.