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

bug ? addToken not work when XRegExp(pattern: RegExp) #240

Open
bluelovers opened this issue Apr 24, 2018 · 4 comments
Open

bug ? addToken not work when XRegExp(pattern: RegExp) #240

bluelovers opened this issue Apr 24, 2018 · 4 comments

Comments

@bluelovers
Copy link

bluelovers commented Apr 24, 2018

bug ? addToken not work when XRegExp(pattern: RegExp)

import addSupportToXRegExp, { IOptions } from 'xregexp-plugin-hanzi-cjk';
import * as XRegExp from 'xregexp';

let options: IOptions = {
	// set a flag if u only wanna trigger for this plugin, default is auto enable
	flags: '',
};

console.log(XRegExp.version);

// if didn't set xr, it is XRegExp
const xr1 = addSupportToXRegExp(null, options);
//const xr2 = addSupportToXRegExp(XRegExp, options);
//console.log('xr1 = xr2 = XRegExp', xr1 === xr2);

let r1 = '(の|像)';
let r2 = /(の|像)/;

// work when input is string
console.log(xr1(r1));
console.log(xr1(r1).test('象'));
console.log(xr1(r1).test('的'));

// fail when input is regexp
console.log(xr1(r2));
console.log(xr1(r2).test('象'));
console.log(xr1(r2).test('的'));
4.1.1
{ /([の之的]|[像象])/ xregexp: { captureNames: null, source: '(の|像)', flags: '' } }
true
true
{ /(の|像)/ xregexp: { captureNames: null, source: null, flags: null } }
false
false
@josephfrazier
Copy link
Collaborator

I think this is expected (or at least, somewhat documented) behavior. From

xregexp/src/xregexp.js

Lines 544 to 545 in 956747f

* // Providing a regex object copies it. Native regexes are recompiled using native (not XRegExp)
* // syntax. Copies maintain extended data, are augmented with `XRegExp.prototype` properties, and
:

Native regexes are recompiled using native (not XRegExp) syntax

You should be able to work around it by using xr1(r2.source) to instead of xr1(r2).

@bluelovers
Copy link
Author

bluelovers commented Apr 24, 2018

@slevithan
Copy link
Owner

slevithan commented Apr 25, 2018

Yeah, as @josephfrazier said, this is intentional and documented (though perhaps the documentation could be more explicit). XRegExp has two modes: it either takes a regex pattern string with optional flags, or it makes a copy of an existing regex object you give it (rather than reinterpreting the regex object with XRegExp syntax), in which case flags are not accepted. If you want to use the source of an existing regex object as your pattern string, use <regexp>.source as @josephfrazier mentioned. Aside: Using XRegExp(/regex/) to copy a regex will extend the copy with XRegExp prototype methods/properties.

The way this works is that when you use XRegExp to copy regexes, it always uses RegExp under the hood. This improves perf and avoids unexpected outcomes where the result would not be a copy at all (especially when custom syntax tokens come into play).

Note that XRegExp's behavior for copying regexes was created prior to ES6. ES6 changed the handling of copying regexes using the RegExp constructor by allowing you to change regex flags in the process, which XRegExp does not (currently) support. This is not an especially useful feature (I think the only common use case is adding /g, and XRegExp offers globalize just for this), but nevertheless it's probably worth following ES6 by also allowing flags to be changed when using the XRegExp constructor to copy regexes. But adopting this feature would probably mean needing to always reparse with XRegExp, even if only to avoid inconsistent handling. Adopting the feature is doable even though XRegExp non-native flags can actually change the source of a regex, because XRegExp (as of v3.0.0) stores the pre-compilation source and flags (as <xregexp>.xregexp.source, separate from <xregexp>.source). You'd have to use that (in combination with the new flags) to make a copy.

@josephfrazier
Copy link
Collaborator

does official xregexp will do something like?

https://github.com/bluelovers/xregexp-plugin-hanzi-cjk/blob/0590c2f04e192f6968653f3b63b612ddd6acece7/index.ts#L73

Based on what @slevithan said above, it sounds like we want to preserve the performance characteristics of not reparsing RegExp objects passed into XRegExp(). You could always write a helper function that does this for you, of course.

and how can post code like u do?

Hmm, it seems like you did it correctly: https://blog.github.com/2017-08-15-introducing-embedded-code-snippets/

Maybe it only works for links to the repo being commented on...

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

No branches or pull requests

3 participants