[rekonq] Re: Review Request: Text highlighting in the url bar simplified

[Benjamin Poulain]

> On 2010-11-19 22:58:49, Jon Ander Peñalba wrote:
> > I think that this solves the problem. I don't know if I've added all the symbols that cause trouble, but if not, it's really simple to update :)

In my opinion, it is just the wrong way to solve the problem. You still have not addressed the case where you match "XX" in a string of "XXXXXXXXXXXXX".

In you last patch, you replace special symbols, but those can appear in an URL, you want to match them. If you want to escape all the control character in a meaningful way, your code will end up being bigger than the previous code.

The objective of you patch is "Text highlighting in the url bar simplified", the new code is neither simplified, neither correct.


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100154/#review365
-----------------------------------------------------------


On 2010-11-19 22:56:28, Jon Ander Peñalba wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100154/
> -----------------------------------------------------------
> 
> (Updated 2010-11-19 22:56:28)
> 
> 
> Review request for rekonq.
> 
> 
> Summary
> -------
> 
> I've simplified text highlighting. I think the behaviour hasn't changed, but a second opinion is welcome :)
> 
> I've removed the Qt::escape (and updated the test accordingly) because I find it useless, but if it needs to be there for any reason there's no problem in putting it back.
> 
> 
> Diffs
> -----
> 
>   src/tests/listitem_test.cpp fc0b62e 
>   src/urlbar/listitem.cpp a0462e7 
> 
> Diff: http://git.reviewboard.kde.org/r/100154/diff
> 
> 
> Testing
> -------
> 
> The 'listitem_test' test passes.
> 
> 
> Thanks,
> 
> Jon Ander
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/rekonq/attachments/20101120/1848009c/attachment.htm