Merge lp://staging/~julien-spautz/cable/markup-refactor-and-better-urls into lp://staging/cable

Proposed by Julien Spautz
Status: Merged
Merged at revision: 136
Proposed branch: lp://staging/~julien-spautz/cable/markup-refactor-and-better-urls
Merge into: lp://staging/cable
Diff against target: 790 lines (+277/-384)
12 files modified
CMakeLists.txt (+9/-5)
src/Cable.vala (+0/-10)
src/Tests/Tests.vala (+0/-32)
src/Tests/Utils.vala (+0/-120)
src/Utils/ColorCodeParser.vala (+143/-0)
src/Utils/UpMarker.vala (+31/-0)
src/Utils/UrlUpMarker.vala (+44/-0)
src/Utils/Utils.vala (+2/-152)
tests/TestColors.vala (+0/-36)
tests/TestExample.vala (+0/-22)
tests/TestMain.vala (+7/-7)
tests/TestUpMarker.vala (+41/-0)
To merge this branch: bzr merge lp://staging/~julien-spautz/cable/markup-refactor-and-better-urls
Reviewer Review Type Date Requested Status
Cable Developers Pending
Review via email: mp+185626@code.staging.launchpad.net

Description of the change

Refactored the color code markup function and copied terminal's URL regexes.

To post a comment you must log in.
Revision history for this message
982c80311320c1b (alexander-wilms) wrote :

What kind of URLs didn't work before?

Revision history for this message
982c80311320c1b (alexander-wilms) wrote :

google.com doesn't work, http://google.com does

Revision history for this message
Julien Spautz (julien-spautz) wrote :

> google.com doesn't work, http://google.com does

google.com probably should work, as it would highlight a lot of non URLs.

AFAIK there were some problems with more exotic URLs. Valadoc URLs were only partially highlighted because they contained a '!' (I fixed it, but probably introduced some other bugs in the process, because I don't even understand our current regex, which is never a good thing...).

The new regexes are a little more sophisticated and should match URLs more accurately. Also there's no point in maintaining two different implementations, so using the same as pantheon terminal will benefit both projects.

I'll also add a bunch of unit test to make sure we don't break existing behaviour in case we decide to modify the regexes.

Revision history for this message
Julien Spautz (julien-spautz) wrote :

s/should/shouldn't/

Revision history for this message
982c80311320c1b (alexander-wilms) wrote :

Typing bug 1234 in #vala on GimpNet should result in a bot posting a red link. Instead, the markup is visible

Revision history for this message
982c80311320c1b (alexander-wilms) wrote :

All unit tests pass

Revision history for this message
Julien Spautz (julien-spautz) wrote :

So it seems to work in certain channels, like #test and #vala.

Seems like other channels disabled it, if that's possible.

Revision history for this message
Julien Spautz (julien-spautz) wrote :

Except for the new regexes, the behaviour of the code didn't change, so if something doesn't work, it's not a regression but a bug in the original implementation, which should be fixed in a separate branch anyway.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: