Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(738)

Issue 29548581: Issue 4128, 5138 - Add Parser and Serializer implemented in C++

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month ago by sergei
Modified:
1 week, 1 day ago
Reviewers:
Wladimir Palant, hub
CC:
Felix Dahlke
Base URL:
https://github.com/adblockplus/adblockpluscore.git
Visibility:
Public.

Description

- move the check whether a template parameter of `intrusive_ptr` inherits `ref_counted` in `static_assert` in the class definition in order to use it with yet incomplete class, e.g. `class Subscription; typedef intrusive_ptr<Subscription> SubscriptionPtr;` - add `addRef = true` argument to `intrusive_ptr::reset`, so it has the same functionality as the constructor. - add `String::toInt64` and `String::toUInt64` parsing the string into an integer - add a very basic `template<typename Type> Type AdaptValue(const String& value)` function allowing to easily convert a string value into a desired `Type`. - add `DependentString TrimSpaces(const String& value)` removing ASCII space characters on both sides of the argument - add `std::pair<DependentString, DependentString> SplitString(const String& value, String::size_type separatorPos)` splitting the string into two parts without a separator character - don't expose in JS API `Subscription::Serialize` - remove `Subscription::SerializeFilters` - add JS _FilterStorage_Parser bound to C++ Parser and is aliased to require("filterStorage").Parser. - add JS _FilterStorage_Serializer bound to C++ `Serializer` and is aliased to require("filterStorage").Serializer - add C++ only `Subscription` factory method `Subscription::FromProperties(const KeyValues& properties)`, `Subscription::KeyValues` and constructor for `Subscription` classes accepting `KeyValues` allowing to construct a subscription from pairs of subscription properties according to the `url` property (either `DownloadableSubscritpion` or `UserDefinedSubscription`). - make the constructor of `Subscription` protected - add C++ only version of `Subscription::AddFilter(Filter&)` adding a filter to the end of subscription filters without notifications. - add C++ only `const Filters& Subscription::GetFilters() const`, for present it's used by `Serializer`. - rename `Subscription::Serialize` to `Subscription::SerializerProperties` because it serializes only certain subscription properties. It's kept as a member of `Subscription` for the sake of simplicity. - add the const version of `Subscription::As()` - add test/_test-utils.js with `withNAD` reducing the amount of code working with objects from emscripten and with `testEqualObjProperties` comparing properties of emscripten classes objects. - update tests The implementations of both `Parser` and `Serializer` are not complete yet in accordance with the issue. It is the first step having the core functionality, allowing to write tests and start to use them, at least partially. In particular, beside some other things, `Parser` is not connected with `FilterStroage` yet and it has to implicitly affect global `knownFilters` and `knownSubscriptions`, `Serializer` does not provide with generator-like interface, does not do anything keeping a passed subscription unmodified externally during serialization process and is not connected with `FilterEngine` either, and there is no conversion to UTF-8.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+819 lines, -108 lines) Patch
M compiled/String.h View 3 chunks +66 lines, -0 lines 0 comments Download
A compiled/String.cpp View 1 chunk +32 lines, -0 lines 0 comments Download
M compiled/bindings/main.cpp View 5 chunks +16 lines, -6 lines 0 comments Download
M compiled/intrusive_ptr.h View 2 chunks +4 lines, -4 lines 0 comments Download
A compiled/storage/Parser.h View 1 chunk +65 lines, -0 lines 0 comments Download
A compiled/storage/Parser.cpp View 1 chunk +183 lines, -0 lines 0 comments Download
A compiled/storage/Serializer.h View 1 chunk +40 lines, -0 lines 0 comments Download
A compiled/storage/Serializer.cpp View 1 chunk +72 lines, -0 lines 1 comment Download
M compiled/subscription/DownloadableSubscription.h View 2 chunks +2 lines, -2 lines 0 comments Download
M compiled/subscription/DownloadableSubscription.cpp View 1 chunk +18 lines, -5 lines 1 comment Download
M compiled/subscription/Subscription.h View 3 chunks +37 lines, -7 lines 0 comments Download
M compiled/subscription/Subscription.cpp View 5 chunks +45 lines, -29 lines 0 comments Download
M compiled/subscription/UserDefinedSubscription.h View 2 chunks +4 lines, -2 lines 0 comments Download
M compiled/subscription/UserDefinedSubscription.cpp View 3 chunks +25 lines, -4 lines 0 comments Download
M lib/filterStorage.js View 2 chunks +4 lines, -1 line 0 comments Download
A test/_test-utils.js View 1 chunk +81 lines, -0 lines 0 comments Download
M test/subscriptionClasses.js View 6 chunks +125 lines, -48 lines 0 comments Download

Messages

Total messages: 2
sergei
https://codereview.adblockplus.org/29548581/diff/29548701/compiled/subscription/DownloadableSubscription.cpp File compiled/subscription/DownloadableSubscription.cpp (left): https://codereview.adblockplus.org/29548581/diff/29548701/compiled/subscription/DownloadableSubscription.cpp#oldcode25 compiled/subscription/DownloadableSubscription.cpp:25: SetTitle(id); Why is the title set to ID only ...
1 month ago (2017-09-18 19:20:07 UTC) #1
sergei
1 month ago (2017-09-19 09:17:35 UTC) #2
https://codereview.adblockplus.org/29548581/diff/29548701/compiled/storage/Se...
File compiled/storage/Serializer.cpp (right):

https://codereview.adblockplus.org/29548581/diff/29548701/compiled/storage/Se...
compiled/storage/Serializer.cpp:67: auto escapedResult =
EscapeOpeningBracket(filter->GetText());
BTW, what about escaping of '[' only when it's the first character and there is
']' at the end of the line?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5