Skip to content
This repository has been archived by the owner on Oct 16, 2020. It is now read-only.

Nano Highlighter initial implementation #29

Closed
jspenguin2017 opened this issue Dec 12, 2017 · 15 comments
Closed

Nano Highlighter initial implementation #29

jspenguin2017 opened this issue Dec 12, 2017 · 15 comments
Labels
archived This thread was archived, open new issues for similar problems. fixed
Milestone

Comments

@jspenguin2017
Copy link
Member

jspenguin2017 commented Dec 12, 2017

Update: Initial implementation finished. Please open new issues for other problems.

@jspenguin2017 jspenguin2017 added this to the v1.0.1 milestone Dec 12, 2017
@jspenguin2017
Copy link
Member Author

jspenguin2017 commented Dec 12, 2017

Nano's current profile:
image

@jspenguin2017
Copy link
Member Author

uBO's profile:
image

@jspenguin2017
Copy link
Member Author

I might have to use a negative look-ahead, but I'm a bit worried about performance as it will have to scan the text twice.

@jspenguin2017 jspenguin2017 changed the title Nano Highlighter does not take in account of RegExp rules Nano Highlighter bugs mega issue Dec 13, 2017
@jspenguin2017
Copy link
Member Author

jspenguin2017 commented Dec 13, 2017

Negated domains are not properly handled @ameshkov Shouldn't that be | instead of ,?
image
Test cases:

||coinhive.com^$third-party,domain=~cnhv.co,~coinhive.com

Another one:
image

ameshkov added a commit to AdguardTeam/AdguardFilters that referenced this issue Dec 13, 2017
ameshkov added a commit to AdguardTeam/AdguardFilters that referenced this issue Dec 13, 2017
@ameshkov
Copy link

Your syntax highlighter is awesome!

@jspenguin2017 jspenguin2017 changed the title Nano Highlighter bugs mega issue Nano Highlighter bugs mega thread Dec 13, 2017
@jspenguin2017 jspenguin2017 changed the title Nano Highlighter bugs mega thread Nano Highlighter issues mega thread Dec 13, 2017
@jspenguin2017
Copy link
Member Author

Old opening:

RegExp rules are not properly handled when the RegExp contains $.
image
Test cases:

/\.download\/[0-9]{2,9}\/$/$script,stylesheet,third-party,xmlhttprequest
/\.faith\/[0-9]{2,9}\/$/$script,stylesheet,third-party,xmlhttprequest
/\.link\/[0-9]{2,9}\/$/$script,stylesheet,third-party,xmlhttprequest
/\.loan\/[0-9]{2,9}\/$/$script,stylesheet,third-party,xmlhttprequest
/\.lol\/[0-9]{2,9}\/$/$script,stylesheet,third-party,xmlhttprequest
/\.men\/[0-9]{2,9}\/$/$script,stylesheet,third-party,xmlhttprequest
/\.online\/[0-9]{2,9}\/$/$script,stylesheet,third-party,xmlhttprequest
/\.party\/[0-9]{2,9}\/$/$script,stylesheet,third-party,xmlhttprequest
/\.racing\/[0-9]{2,9}\/$/$script,stylesheet,third-party,xmlhttprequest

/http://[a-zA-Z0-9]+\.[a-z]+\/.*(?:[!"#$%&'()*+,:;<=>?@/\^_`{|}~-]).*[a-zA-Z0-9]+/$script,third-party,domain=keezmovies.com|redtube.com|tube8.com|tube8.es|tube8.fr|www.pornhub.com|youporn.com

Also, these rules are in EasyList but Nano Highlighter marks them as invalid: Those options are not accepted by Nano.

||ngohq.com/images/ad.jpg$~collapse
@@||oload.tv^$genericblock,generichide
@@||openload.co^$genericblock,generichide
@@||streamango.com^$genericblock,generichide
@@||spiegel.de^$genericblock,generichide

image


, in script:inject arguments should be treated as operator just like other ,.
That's how it's parsed.


Handle folding of !#if preprocessor directive. AdguardTeam/AdguardBrowserExtension#917
Ace's doc on this is pretty non-existent... Might have to read deep into their code.


Something is not right with Traditional Chinese locale:
#40 (comment)

@jspenguin2017
Copy link
Member Author

This is fixed:

RegExp rule highlighting is not properly fixed:
image
Test rules:

/test$/

@ameshkov
Copy link

JFYI,

I've just figured that Ace provides an easy conversion from TextMate language to Ace rules:
https://github.com/ajaxorg/ace/wiki/Importing-.tmtheme-and-.tmlanguage-Files-into-Ace

The thing is that a lot of filters maintainers use external text editors (besides the filters editor inside of a blocker), and most of the popular text editors (Sublime, Atom, VS Code) support TextMate language extensions, so it might useful to use a TM language as the main source for the rules.

I'm planning to experiment with this during the holidays, but anyway, I guess this might be useful to you if you decide to do the same.

Here's a simple VS code language plugin:
https://github.com/ameshkov/VscodeAdblockSyntax

TM language file:
https://github.com/ameshkov/VscodeAdblockSyntax/blob/master/syntaxes/adblock.tmLanguage.json

It's rather simple (as I've just made it) and can only highlight classic elemhide rules:

@jspenguin2017
Copy link
Member Author

Filter rules are different than other programming language, it's not something you'd code 100 lines and test them together, I find myself code them in Nano's IDE, test them right away, then paste them into my filter list. Nano will validate and lint it right away with the existing filter parser, which can be a bit difficult to implement in Atom plugin.

At first, I tried to make a plugin for Atom, but then I couldn't find the proper documentation. Maybe I was missing something...

@ameshkov
Copy link

Filter rules are different than other programming language

Take a look at all the languages VS code supports, some of them are very different from imperative languages we are used to:
https://github.com/Microsoft/vscode/tree/master/extensions

Nano will validate and lint it right away with the existing filter parser, which can be a bit difficult to implement in Atom plugin.

I suppose, that validation and syntax highlighting are different types of tasks indeed. Every editor provides its own way for that. For instance, VS code does it via "language server" extensions. Some obvious mistakes can be highlighted using regular expressions, though.

Regarding Atom, it also supports the TextMate theme conversion:
http://flight-manual.atom.io/hacking-atom/sections/converting-from-textmate/

@jspenguin2017
Copy link
Member Author

jspenguin2017 commented Feb 10, 2018

Unless you guys have constructive comments, just don't post it. It's not getting anywhere. I can't change gorhill's decisions, no matter I agree with him or not.

@NanoAdblocker NanoAdblocker deleted a comment from elypter Feb 10, 2018
@NanoAdblocker NanoAdblocker deleted a comment Feb 10, 2018
@NanoAdblocker NanoAdblocker deleted a comment Feb 10, 2018
@jspenguin2017
Copy link
Member Author

jspenguin2017 commented Feb 10, 2018

I don't mind civil discussions, but the discussion is getting not that civil and off topic from development of Nano.

@jspenguin2017 jspenguin2017 changed the title Nano Highlighter issues mega thread Nano Highlighter initial implementation Mar 2, 2018
@jspenguin2017 jspenguin2017 removed the META label Mar 2, 2018
@jspenguin2017
Copy link
Member Author

jspenguin2017 commented Mar 2, 2018

Added preprocessor highlighting: AdguardTeam/AdguardBrowserExtension#917

@NanoAdblocker NanoAdblocker locked as resolved and limited conversation to collaborators Mar 2, 2018
@NanoAdblocker NanoAdblocker unlocked this conversation Mar 2, 2018
@jspenguin2017
Copy link
Member Author

jspenguin2017 commented Mar 2, 2018

I'll leave this thread open for quick comments, but please report more complicated problems in new issues.

@github-actions github-actions bot added the archived This thread was archived, open new issues for similar problems. label Aug 21, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Aug 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived This thread was archived, open new issues for similar problems. fixed
Projects
None yet
Development

No branches or pull requests

4 participants
@ameshkov @jspenguin2017 and others