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

Throw when converting a scriptlet with path option to uBO syntax #182

Merged
merged 2 commits into from
Jul 11, 2023
Merged

Throw when converting a scriptlet with path option to uBO syntax #182

merged 2 commits into from
Jul 11, 2023

Conversation

gorhill
Copy link
Contributor

@gorhill gorhill commented Jul 7, 2023

uBO's scriptlet syntax does not support AdGuard's [path=...] option.

To prevent conversion to invalid filter, the conversion will error when attempting to convert an AdGuard scriptlet with a path option present.

Related issue:

uBO's scriptlet syntax does not support AdGuard's `[path=...]`
option.

To prevent conversion to invalid filter, the conversion will
error when attempting to convert an AdGuard scriptlet with a
path option present.

Related issue:
- AdguardTeam/tsurlfilter#65
@slavaleleka
Copy link
Contributor

@gorhill could you please also add some tests for it?

@gorhill
Copy link
Contributor Author

gorhill commented Jul 7, 2023

Not sure how is thebest way to test this. I implemented using a throw because the code in there is ready to deal with exception:

https://github.com/AdguardTeam/FiltersCompiler/blob/master/src/main/converter.js#L33-L44

In which case if I understand correctly, the filter will be dropped.

The test for scriptlets with path modifier is at:

https://github.com/AdguardTeam/FiltersCompiler/blob/master/src/test/converter.test.js#L573-L581

But not sure how to test for expected exception.

@gorhill
Copy link
Contributor Author

gorhill commented Jul 7, 2023

I think I understand better how this is supposed to work, I will modify the fix.

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.

5 participants