Implement separateGroups option #183
Conversation
|
I've made the necessary fixes, but I can't get the tests to complete; and it always stops at the same point... Should I go ahead and push the changes? I'm sure the separate groups tests will still pass, but we can't be sure until I push the code here. |
|
@Mottie Yes, please push it. We then see why this error appears. |
|
Grrr, eslint isn't showing me that the complexity level issue. |
|
@Mottie I've successfully ran the build on the master branch with the same modules like on groups-restore. So I was sure that it's an issue of the changes. Then I played around with the test around the last successful one and gotcha: <!doctype html>
<html>
<head>
<meta charset="utf-8">
<title>mark.js manual test (dev purposes)</title>
<style>
mark {
background: yellow;
}
</style>
</head>
<body>
<div id="context"></div>
<script src="../node_modules/jquery/dist/jquery.min.js"></script>
<script src="../dist/mark.js"></script>
<script>
const $ctx = $("#context");
$ctx.load("fixtures/basic/duplicate-context.html", function() {
var $ctx1 = $ctx.find('.basic-duplicate-context > div:first-child');
var $ctx2 = $ctx.find('.basic-duplicate-context > div:last-child');
var ctx1Called = 0;
var ctx2Called = 0;
function done(){
console.log('DONE');
}
new Mark([$ctx1[0], $ctx1[0]]).mark('test', {
'diacritics': false,
'separateWordSearch': false,
'filter': function(){
ctx1Called++;
// return false. Otherwise matches would become wrapped and no
// further matches would be found. Therefore no further filter
// calls would be done
return false;
},
'done': function() {
new Mark([$ctx2[0], $ctx2.find('span')[0]]).mark('test', {
'filter': function(){
ctx2Called++;
// return false. Otherwise matches would become wrapped
// and no further matches would be found. Therefore no
// further filter calls would be done
return false;
},
'done': function(){
done();
}
});
}
});
});
</script>
</body>
</html> |
|
Is nothing highlighted in that example? I ran it on the master branch and nada... https://jsfiddle.net/j0jdyama/37/ Edit: I mean, I don't understand what it means that the matches will become wrapped. It works fine if I return |
|
@Mottie No highlights are expected in this test. The filter callback function is always returning However, if I open the above mentioned manual.html page in the browser I'm getting a toolbar if I want to terminate the script and then I get:
So the browser freezes, and this is the issue I was talking about, not that no highlights are done. |
Mottie
force-pushed
the
groups-restore
branch
from
4d7d2f6
to
10065cf
Compare
|
@Mottie Are you done from your side? |
|
Yes 😸 |
|
Can you please also answer my questions?
|
The
The |
|
@Mottie As this replaces the |
|
I mean the |
|
@Mottie If you can achieve the same effect with plain This will then be available with the next major release. |
As it stands now, you can't achieve the same effect easily. You would need to include the match index in the |
|
Ping @julmot... what should I do? |
|
@Mottie Sorry for not answering earlier. As far as I understand it would be enough to modify the parameters of the each and filter callbacks. Since this would mean that it's a backward incompatible change it would require a major version. Since v9.0.0 is already in progress with the diacritics feature, I'd suggest to merge this PR and implement the remaining callback modifications once the diacritics thing is completed. Do you agree? |
|
Yes, we can remove the |
|
@Mottie I have planned to release the v9 release as it is with the following features:
Therefore, my question to you is what we need to do in order to release this? We should normalize |
|
Oh, I thought the plan was to remove |
|
@Mottie So, as far as I understand, the Therefore, it's safe to drop |
|
Yes! 😸 |
According to #132
Follow-up from #136
@Mottie I've now restored your original PR. Since then, I've added new ESLint config properties that caused it to fail, since the
wrapMatchesmethod is now too complex. Therfore I added ignore comments temporarily. Do you have a suggestion of making this for easy to read and understand?Is this a replacement of the
ignoreGroupsoption? Could you please describe the change in a short explanation for the docs?