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

Fixes #248 Adds tieWith #250

Merged
merged 2 commits into from
Jun 5, 2018
Merged

Fixes #248 Adds tieWith #250

merged 2 commits into from
Jun 5, 2018

Conversation

ivan-kleshnin
Copy link
Contributor

@ivan-kleshnin ivan-kleshnin commented Jun 3, 2018

An attempt to implement #248

The problem I have is a coverage drop from 100% to 99.8%.

  278 passing (184ms)


=============================== Coverage summary ===============================
Statements   : 99.81% ( 523/524 )
Branches     : 99.4% ( 165/166 )
Functions    : 100% ( 138/138 )
Lines        : 99.81% ( 523/524 )
================================================================================

I guess that's because of the following !!! (condition branch).

_.tieWith = function(separator) {
  assertString(separator);
  return this.map(function(args) {
    assertArray(args);
    if (args.length) { // !!!
      assertString(args[0]);
      var s = args[0];
      for (var i = 1; i < args.length; i++) {
        assertString(args[i]);
        s += separator + args[i];
      }
      return s;
    } else { // !!!
      return "";
    }
  });
};

I dunno how to create the case where args would be [] :(
Hope for your advice.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 99.71% when pulling a078abe on Paqmind:master into cfe635e on jneen:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 99.71% when pulling a078abe on Paqmind:master into cfe635e on jneen:master.

@coveralls
Copy link

coveralls commented Jun 3, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling ace42d8 on Paqmind:master into cfe635e on jneen:master.

@wavebeem
Copy link
Collaborator

wavebeem commented Jun 3, 2018

Hmm, for the coverage issue, what about this?

Parsimmon.of([]).tie().tryParse("")

Thanks for the PR by the way! 😄

@ivan-kleshnin
Copy link
Contributor Author

Updated.

@wavebeem
Copy link
Collaborator

wavebeem commented Jun 4, 2018

Thanks! I'll merge this tonight when I have time to update the changelog and do the release

@wavebeem wavebeem merged commit 6a93e5b into jneen:master Jun 5, 2018
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.

3 participants