Issue 6923 - Simplify parsing of metadata

Issue 6923 - Simplify parsing of metadata

[Sebastian Noack, 2018-09-13 16:10:27]

Sorry, we changed the semantics in python-abp again [1], now stopping to parse
any further metadata as soon as we see any line that doesn't parse as metadata,
but in order to keep parsing metadata after bogus keys like "Last modified" we
relaxed the regular expression.

These changes make Adblock Plus behave consistently, and FWIW simplify the logic
a bit here.

(I have to run, but if you insist I can file an issue later.)

[1]: https://issues.adblockplus.org/ticket/6950

[Manish Jethani, 2018-09-14 05:25:44]

On 2018/09/13 16:10:27, Sebastian Noack wrote:
> Sorry, we changed the semantics in python-abp again [1], now stopping to parse
> any further metadata as soon as we see any line that doesn't parse as
metadata,
> but in order to keep parsing metadata after bogus keys like "Last modified" we
> relaxed the regular expression.
> 
> These changes make Adblock Plus behave consistently, and FWIW simplify the
logic
> a bit here.
> 
> (I have to run, but if you insist I can file an issue later.)
> 
> [1]: https://issues.adblockplus.org/ticket/6950

Is there a specific reason why we don't want to parse a line like "! : foo" as
metadata (with a blank key)?

Yes, I think you should file an issue. Maybe it's time to simplify the regular
expression even further and remove the trailing whitespace from the value while
we're at it.

[Sebastian Noack, 2018-09-14 14:30:27]

On 2018/09/14 05:25:44, Manish Jethani wrote:
> Is there a specific reason why we don't want to parse a line like "! : foo" as
> metadata (with a blank key)?

Indeed, this would simplify the regexp quite a bit. Done.

> Yes, I think you should file an issue.

Or would you mind if I rather reference issue 6923, and update the description
accordingly, assuming we still land this before 3.4? I think this would make it
easier for testers.

> Maybe it's time to simplify the regular
> expression even further and remove the trailing whitespace from the value
while
> we're at it.

Wouldn't removing trailing whitespaces make the regexp more complex?

[Manish Jethani, 2018-09-16 11:15:09]

On 2018/09/14 14:30:27, Sebastian Noack wrote:
> On 2018/09/14 05:25:44, Manish Jethani wrote:
> > Yes, I think you should file an issue.
> 
> Or would you mind if I rather reference issue 6923, and update the description
> accordingly, assuming we still land this before 3.4? I think this would make
it
> easier for testers.

Sure, works for me.

> > Maybe it's time to simplify the regular
> > expression even further and remove the trailing whitespace from the value
> while
> > we're at it.
> 
> Wouldn't removing trailing whitespaces make the regexp more complex?

I was thinking like `/^\s*!([^:]*):(.*)/`, and then we use String.trim() on both
the keyword and the value. I'm not sure if it's better or worse.

[Manish Jethani, 2018-09-16 11:28:11]

LGTM