Skip to content
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

Allow additional ignored classes when inferring tags in DebugTree #362

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

drewhamilton
Copy link

Fixes #180 on a per-Tree basis.
Replaces #314 given the new internal tag-determination logic.

Add DebugTree#ignoreForTagging(Class) to allow more classes to be ignored when the tag is inferred from the stacktrace. This is useful for anyone needing to wrap Timber, e.g. for use in a Java-only module.

Also changes fqcnIgnore from a list into a set because only its contains method is used.

@yhartanto
Copy link

Do we need to add the additional exception list post instantiation? Might be better to just offer a constructor param to allow consumer send class list, that way the list can be immutable still.

From a consumer perspective, typically you initialize Timber once and don't touch it again.

@yhartanto
Copy link

yhartanto commented Jun 24, 2020

open class DebugTree(vararg ignoredClasses: Class<Any>) : Tree() {
    private val fqcnIgnore =  listOf(
        Timber::class.java.name,
        Timber.Forest::class.java.name,
        Tree::class.java.name,
        DebugTree::class.java.name
    ).plus(ignoredClasses)

@drewhamilton
Copy link
Author

Yeah, that's a better API. Happy to update if there's interest in merging this feature.

@tpucci
Copy link

tpucci commented Jul 1, 2020

I am very interested by this :)

@yhartanto
Copy link

Yeah me too. 👍

@drewhamilton
Copy link
Author

Converted to a constructor parameter. The semantics are a bit weird now though: it's not obvious when reading DebugTree(SomeClass::class.java) that the tree would ignore "SomeClass" for tags. I wonder if a static factory like DebugTree.withIgnoredClasses would be more appropriate.

@yhartanto
Copy link

Converted to a constructor parameter. The semantics are a bit weird now though: it's not obvious when reading DebugTree(SomeClass::class.java) that the tree would ignore "SomeClass" for tags. I wonder if a static factory like DebugTree.withIgnoredClasses would be more appropriate.

I think that is a valid consideration, we can add a static factory to make the API more explicit.

@TurKurT656
Copy link

What if we wrap Timber logs inside kotlin top level functions?

LogExtension.kt:

fun logD("message") {
    Timber.d(message)
}

Then if we want to ignore it we do not access to the class. However there is an auto generated class named LogExtensionKt that is hidden from kotlin files. So what we can do about it?
Maybe if we accept ignored classes as strings we can solve this problem but I'm not sure if its a good thing to do.

@Chesteer89
Copy link

Any update on this?

@dyguests
Copy link

Any update?

@dyguests
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optional call stack depth parameter
6 participants