Opened on 01/13/2015 at 01:12:33 PM
Last modified on 02/04/2015 at 03:27:35 AM
#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.
Attachments (0)
Note: See
TracTickets for help on using
tickets.
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.