Opened 5 years ago

Last modified 5 years ago

#1791 new change

Move FilterEngine::ContentType and related stuff into separate files.

Reported by: sergz Assignee:
Priority: Unknown Milestone:
Module: Libadblockplus Keywords:
Cc: oleksandr, eric@…, fhd Blocked By:
Blocking: Platform: Unknown
Ready: no Confidential: no
Tester: Verified working:
Review URL(s):



During the discussion on IRC we decided to try as is and then change it in case of necessity. The code review has shown that it should be changed.

What to change

Move FilterEngine::ContentType declaration and helper functions into separate file, so we can easily include only these two files into IE Addon project without linking with heavy libadblockplus and without over-complicating libadblockplus by additional shared project.

Change History (1)

comment:1 Changed 5 years ago by fhd

I wouldn't be against moving ContentType and the conversion functions out of FilterEngine. However, not linking to libadblockplus would require those conversion functions to be implemented in the header - and I think that's going too far.

What the plugin gets, and always got, is a content type as string. I can see why we want to use an enum rather, and the one in libadblockplus happens to do what we want. We can either include/link or copy it.

I do think it makes sense not to use libadblockplus in ABP for IE (even when not considering memory overhead) - that's why it used to be that way.

So, while neither option is great, I think I'd be leaning towards the copy. If we copy ContentType.h/cpp as-is (maybe even automatically during the build), it seems alright. As long as it's made obvious what happens, that is.

Note: See TracTickets for help on using tickets.