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

Add matchDebug() for Cliqz #2162

Merged
merged 1 commit into from
Aug 12, 2021
Merged

Conversation

mjethani
Copy link
Contributor

@mjethani mjethani commented Aug 12, 2021

This patch adds a matchDebug() implementation for Cliqz.

@mjethani mjethani requested a review from remusao as a code owner August 12, 2021 14:50
@mjethani mjethani changed the title Add matchDebug() for uBlock Origin and Cliqz Add matchDebug() for Cliqz Aug 12, 2021
@mjethani
Copy link
Contributor Author

Sorry I just noticed #2160. I have made this only for Cliqz now.

@remusao remusao added the PR: Internal 🏠 Changes only affect internals label Aug 12, 2021
url,
sourceUrl: frameUrl,
type,
})).filter || null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

By default the original filter might not be kept around (but the engine will try to recreate it from internal state. If we know we are in debug mode we can specify it to the FiltersEngine.parse method above:

  static parse(rawLists, enableCompression, debug) {
    return new Cliqz(FiltersEngine.parse(rawLists, {
      enableCompression,
      integrityCheck: false,
      loadCosmeticFilters: false,
      debug: debug === true,
    }));
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

url,
sourceUrl: frameUrl,
type,
})).filter || null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also I think filter will be an object and not sure if the toString() will be called implicitly somewhere? If not:

Suggested change
})).filter || null;
})).filter?.toString() || null;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I'm returning the value of rawLine or null.

Copy link
Collaborator

@remusao remusao left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@remusao remusao merged commit 584e1d3 into ghostery:master Aug 12, 2021
@mjethani mjethani deleted the add-ubo-matchdebug branch August 12, 2021 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Internal 🏠 Changes only affect internals
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants