[
https://issues.apache.org/jira/browse/WAVE-275?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13175111#comment-13175111
]
[email protected] commented on WAVE-275:
----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3294/#review4088
-----------------------------------------------------------
Nice work - the framework stuff is great, though this looks like it'd be rather
slow on a client with a large blip document;
can it be made faster? The editor update event should come with flags (content
changes / cursor changed etc) so only ones where the content changed
need to be inspected, and for those, only the content around the cursor (which
should also be available) need be inspected.
/src/org/waveprotocol/wave/client/doodad/link/Link.java
<https://reviews.apache.org/r/3294/#comment9185>
Can this list be placed in EscapeUtils?
It feels better to have the dependency that way - things that want Link can
have EscapeUtils, whereas things might want EscapeUtils without getting Link.
/src/org/waveprotocol/wave/client/editor/impl/AutoLinker.java
<https://reviews.apache.org/r/3294/#comment9188>
this shouldn't need to be a member variable - pass it to mabeSetOrUpdate
and descendants as a parameter, that way nothing has to be update-scoped.
/src/org/waveprotocol/wave/client/editor/impl/AutoLinker.java
<https://reviews.apache.org/r/3294/#comment9189>
nit: lots of empty lines here.
/src/org/waveprotocol/wave/client/editor/impl/AutoLinker.java
<https://reviews.apache.org/r/3294/#comment9186>
StringSet is advised here for speed -
s = CollectionUtils.copyStringSet(...);
/src/org/waveprotocol/wave/client/editor/impl/AutoLinker.java
<https://reviews.apache.org/r/3294/#comment9192>
What's this part for? it seems to be for matching schemes by suffix - e.g.
thisisamailto:// will match mailto?
Should these be matched, or just <whitespace/lines>mailto://
/src/org/waveprotocol/wave/client/editor/impl/AutoLinker.java
<https://reviews.apache.org/r/3294/#comment9190>
nit: space between ) { to be consistent with other places.
/src/org/waveprotocol/wave/client/editor/impl/AutoLinker.java
<https://reviews.apache.org/r/3294/#comment9187>
I'd pull out "://" into a static final String - as it's referred to on 133
too.
(plus the magic '3' below can be SEPARATOR.length() instead).
/src/org/waveprotocol/wave/client/editor/impl/AutoLinker.java
<https://reviews.apache.org/r/3294/#comment9191>
feels like weird indentation here - 6 spaces whereas the method above has 4
and the ones above that have 2.
- Patrick
On 2011-12-22 16:43:51, Ali Lown wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/3294/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-12-22 16:43:51)
bq.
bq.
bq. Review request for wave and Yuri Zelikov.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Implements an auto-linker feature as requested.
bq.
bq. This implementation works by splitting the data by the uri seperator "://"
and then attempting to match the scheme to a known (currently hard-coded list)
of schemes. The end is detected by either a space or EOL.
bq.
bq. This implementation allows any character to be before a valid scheme, but
relies upon a space or EOL to find the end.
bq.
bq.
bq. This addresses bug WAVE-275.
bq. https://issues.apache.org/jira/browse/WAVE-275
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. /src/org/waveprotocol/wave/client/StageThree.java 1213039
bq. /src/org/waveprotocol/wave/client/common/safehtml/EscapeUtils.java
1213039
bq. /src/org/waveprotocol/wave/client/doodad/link/Link.java 1213039
bq. /src/org/waveprotocol/wave/client/editor/impl/AutoLinker.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/3294/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Tested manually by running the server and typing in URLs in various forms,
as well as by copy-pasting in.
bq.
bq.
bq. Thanks,
bq.
bq. Ali
bq.
bq.
> Auto-linking
> ------------
>
> Key: WAVE-275
> URL: https://issues.apache.org/jira/browse/WAVE-275
> Project: Wave
> Issue Type: New Feature
> Components: Web Client
> Reporter: Daniel Danilatos
> Assignee: Ali Lown
> Labels: StarterProject
>
> Implement auto-linking on the client. If the user types something that looks
> like a link, we should annotated it with "link/auto". This should probably be
> done in a listener to the editor's rate-limited udpate event, to avoid
> performance issues.
> This would be a pretty simple self contained starter project.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
