AdguardTeam / VscodeAdblockSyntax

Comprehensive extension to manage Adblock Syntax in VSCode: syntax highlighter, linter and much more. Available on the VSCode Marketplace.
https://marketplace.visualstudio.com/items?itemName=adguard.adblock
MIT License
70 stars 8 forks source link

Add formatter option to the VSCode extension #98

Open ameshkov opened 1 year ago

ameshkov commented 1 year ago

At this point this is an issue for discussing how the formatter can be implemented and how exactly the filter lists and individual rules should be formatted / sorted.

My initial idea is to sort the filter lists so that all rules for a given website were collected together. Something like this:

google.com###banner
||example.org^$domain=google.com
||google.com/test

At the same time we should take comments into account. Maybe allow grouping rules into a single section using special comments.

Anyways, lets discuss.

CC @AdguardTeam/filters-maintainers @maximtop @scripthunter7

scripthunter7 commented 1 year ago

@ameshkov

At this point this is an issue for discussing how the formatter can be implemented

I see two main ways of implementation:

A while ago I collected a basic concept for the rule orderer. You can find it here: https://github.com/scripthunter7/adblock-rule-orderer

I would separate the following steps:

My initial idea is to sort the filter lists so that all rules for a given website were collected together

I also thought about grouping the rules by websites, but what should happen if a filtering rule contains several different domains? For example:

example.com,example.net###banner
/script-to-block.js^$script,domain=example.com|example.org|~example.net

or if one rule belongs to the scope of a preprocessor directive, while the others don't:

example.com###banner-1
example.com###banner-2

!#if (adguard_ext_safari || adguard_app_ios || adguard_ext_android_cb)
@@/script-to-block^$script,domain=example.com
! ...
! other rules
!#endif

Should these rules be moved to some kind of mixed category?

scripthunter7 commented 1 year ago

We also should consider to keep the formatting in sync with the planned code guidelines: https://github.com/AdguardTeam/CodeGuidelines/issues/16

maximtop commented 1 year ago

Not clear how to resolve cases when one comment is related to a group of rules

ameshkov commented 1 year ago

@maximtop we can provide special rules for that, something like this:

! block start

! block end
scripthunter7 commented 1 year ago

If we introduce such rules, perhaps it would make sense to follow the pattern of preprocessor directives:

!#section
rules
!#endsection
ameshkov commented 1 year ago

IMO, formatter and linter are different things and mixing them together may be confusing.

At the same time, it does not mean that AGLint cannot enforce formatting rules, it can rely on the formatter for that.

scripthunter7 commented 12 months ago

After all, AGTree is the basis, linter and formatter can also be built on it.

It is important to divide the formatter into two categories:

Separate config schemas should be prepared for both categories.

Alex-302 commented 11 months ago

My initial idea is to sort the filter lists so that all rules for a given website were collected together.

We use this approach for some sites. In general it is not a good idea to apply it for all rules, because some rules can be applied to many sites, they can have mirrors (not only with different TLD), and they merged into one large rule.

At the same time we should take comments into account. Maybe allow grouping rules into a single section using special comments.

In some cases we are using Comment Anchors to improve file navigation

! SECTION: Cookies - Popular rules
||rule1.com^
||ruleN.com^
! NOTE: Popular rules end ⬆️
! !SECTION: Cookies - Popular rules
How it looks: ![image](https://github.com/AdguardTeam/VscodeAdblockSyntax/assets/8361299/8eb1971d-ef40-4a8b-ac83-a9d9f0c9c1da)

Automatically creating groups will not improve convenience. As written above - there may (will) be problems with existing groups, directives and hints. Previously never felt the need to create a separate group for each site. This may be handy for the hotfixes filter, but not for the main filters.

ameshkov commented 11 months ago

The problem with the current structure in AdguardFilters is that it's hard to understand where to add a new rule unless you already have lots of experience working with it. I don't think we should take it as an example. Instead of that, let's think what structure would be reasonable to use if we were creating a new list from scratch.

scripthunter7 commented 7 months ago

IMO, formatter and linter are different things and mixing them together may be confusing.

At the same time, it does not mean that AGLint cannot enforce formatting rules, it can rely on the formatter for that.

@ameshkov ESLint also drops formatting rules: https://eslint.org/blog/2023/10/deprecating-formatting-rules/