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

Tests for RegExp, toString should return correct flags and in correct order #364

Merged
merged 1 commit into from
Dec 9, 2015

Conversation

Xotic750
Copy link
Contributor

@Xotic750 Xotic750 commented Dec 9, 2015

IE8 fails this.
Can be fixed by creating from source and using flags.

  function regExpToString(value) {
    var str = '/' + value.source + '/';
    if (value.global) {
      str += 'g';
    }
    if (value.ignoreCase) {
      str += 'i';
    }
    if (value.multiline) {
      str += 'm';
    }
    if (value.sticky) {
      str += 'y';
    }
    return str;
  }

describe('RegExp', function () {
'use strict';

describe('.toString()', function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

. is for static methods - this is an instance method, so it should use #

@ljharb
Copy link
Member

ljharb commented Dec 9, 2015

Can you also add an it with a bunch of RegExp objects?

@Xotic750
Copy link
Contributor Author

Xotic750 commented Dec 9, 2015

Ok

@Xotic750 Xotic750 force-pushed the regexp branch 2 times, most recently from 3fd2690 to ce03328 Compare December 9, 2015 19:38
@ljharb
Copy link
Member

ljharb commented Dec 9, 2015

it looks like your rebase deleted the code that adds the regexp tests to the HTML files?

@Xotic750
Copy link
Contributor Author

Xotic750 commented Dec 9, 2015

Ah bugger :)

@Xotic750
Copy link
Contributor Author

Xotic750 commented Dec 9, 2015

Fixed

@ljharb ljharb merged commit 9ae8165 into es-shims:master Dec 9, 2015
ljharb added a commit that referenced this pull request Dec 9, 2015
@ljharb
Copy link
Member

ljharb commented Dec 9, 2015

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants