Conversation
dnkoutso
force-pushed
the
dimitris/android-dep
branch
from
796b309 to
3f8ace2
Compare
| private int curCharIndex; | ||
|
|
||
| /** | ||
| * When enabled, span support is disabled. This can be useful for unit tests that don't want |
There was a problem hiding this comment.
might need a bit help to word this better.
There was a problem hiding this comment.
first sentence is redundant with field name. Just keep the second sentence ("This can be useful...").
dnkoutso
force-pushed
the
dimitris/android-dep
branch
from
3f8ace2 to
42ef4ee
Compare
|
|
||
| /** Set whether span support is enabled or not. */ | ||
| @SuppressWarnings("UnusedDeclaration") // Public API. | ||
| public void setDisableSpanSupport(boolean disableSpanSupport) { |
There was a problem hiding this comment.
invert: setSpanSupportEnabled. default to true.
javadoc should indicate default. move javadoc from field to this method.
dnkoutso
force-pushed
the
dimitris/android-dep
branch
from
42ef4ee to
03832d3
Compare
| import static java.util.regex.Matcher.quoteReplacement; | ||
| import static java.util.regex.Pattern.quote; | ||
|
|
||
| public class SimpleEditable implements Editable { |
There was a problem hiding this comment.
final. definitely not public. needs javadoc.
dnkoutso
force-pushed
the
dimitris/android-dep
branch
from
03832d3 to
40375f6
Compare
| * Default is <code>true</code>. | ||
| */ | ||
| @SuppressWarnings("UnusedDeclaration") // Public API. | ||
| public void setSpanSupportEnabled(boolean spanSupportedEnabled) { |
There was a problem hiding this comment.
Have you considered just making this variable final, and having new factory methods for it? There'd be a lot of method proliferation I guess.
I was thinking of something like Phrase.stubSpan().from(...) or Phrase.stubSpanFrom(...) or something. That way you just have a small amount of code to add across a lot of tests.
I'd just like someone to avoid running into this method in their normal Phrase usage.
|
LGTM |
| /** | ||
| * This can be useful for unit tests that don't want to use {@link SpannableStringBuilder} | ||
| * which can cause a stub exception or be no-op. | ||
| * Default is <code>true</code>. |
There was a problem hiding this comment.
I should change that since I switched it, it should say <code>false</code>
dnkoutso
force-pushed
the
dimitris/android-dep
branch
2 times, most recently
from
bdc9261 to
d57d3d8
Compare
|
We should merge this, just noticed we've been using it for a looong time right off this commit. We may want to pursue some sort of "default" span enabled setting that would enable even easier test writing |
dnkoutso
force-pushed
the
dimitris/android-dep
branch
from
d57d3d8 to
ad818ca
Compare
dnkoutso
force-pushed
the
dimitris/android-dep
branch
2 times, most recently
from
a876d9b to
663ca14
Compare
|
@edenman we should get this in for a release |
|
@dnkoutso want to rebase? |
3 participants
This mode unlocks the ability for consumers to disable span support. This enables writing pure unit tests that do not depend on
SpannableStringBuilderwhich otherwise would require Robolectric to run and make the tests much much slower.Fixed all warnings too.
@loganj @ChrisRenke @rjrjr