-
Notifications
You must be signed in to change notification settings - Fork 969
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add getDefaultTag() so a Tree can have its own custom tag generation logic #208
base: trunk
Are you sure you want to change the base?
Conversation
* If no tag is explicitly set, then the tree will fall back to this tag. | ||
* @return The default tag for this tree | ||
*/ | ||
protected String getDefaultTag() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's annotate with @Nullable
@@ -391,14 +391,24 @@ private Timber() { | |||
public static abstract class Tree { | |||
final ThreadLocal<String> explicitTag = new ThreadLocal<>(); | |||
|
|||
String getTag() { | |||
private String getTag() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was probably package private to remove the creation of synthetic accessor methods.
@@ -467,6 +467,58 @@ protected String formatMessage(String message, Object[] args) { | |||
.hasDebugMessage("TimberTest", "Test formatting: Test message logged. 100"); | |||
} | |||
|
|||
@Test public void logsWithCustomTag() { | |||
Timber.plant(new Timber.DebugTree() { | |||
@Override |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should be on the same line
|
||
Timber.d("Test with custom tag"); | ||
assertLog() | ||
.hasDebugMessage("CUSTOMTAG", "Test with custom tag"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
continuation indent is 4 spaces and this could also easily get into one line
|
||
@Test public void logsWithCustomTagOverridden() { | ||
Timber.plant(new Timber.DebugTree() { | ||
@Override |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
Timber.tag("NewTag").d("Tag manually set"); | ||
|
||
assertLog() | ||
.hasDebugMessage("NewTag", "Tag manually set"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
assertLog() | ||
.hasDebugMessage("CUSTOMTAG", "multiple tags") | ||
.hasDebugMessage("DIFFERENTTAG", "multiple tags"); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: no new line needed
Timber.d("multiple tags"); | ||
|
||
assertLog() | ||
.hasDebugMessage("CUSTOMTAG", "multiple tags") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's do continuation indent of 4
|
||
@Test public void logsWithMultipleTreesMultipleTags() { | ||
Timber.plant(new Timber.DebugTree() { | ||
@Override |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
}); | ||
|
||
Timber.plant(new Timber.DebugTree() { | ||
@Override |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
@vanniktech thanks for reviewing this. I don't plan on working on this guy anymore since I don't use Timber or really do much Android development at all. I'll leave it open if you want to contribute changes to it, otherwise feel free to close it out. |
- Annotate getDefaultTag() as @nullable - General clean-up/styling changes
@vanniktech OK I guess I lied, I found a little free time and was still thinking about this for some reason & your feedback wasn't too much, I made changes according to your comments and pushed 'em up |
Closes #118
This allows a Tree to set its own tag by overriding the method getDefaultTag(). Currently, a Tree can only have a tag set if Timber.tag is explicitly called.
Note that the current tag behavior still should work the same way - if Timber.tag gets called, that tag will be used over the default.