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):

Description

Background

During the discussion on IRC we decided to try as is and then change it in case of necessity. The code review http://codereview.adblockplus.org/5316782940225536/ 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.