-
-
Notifications
You must be signed in to change notification settings - Fork 390
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
3516 provide occurrence log #3517
base: develop
Are you sure you want to change the base?
Conversation
app/src/main/java/com/keylesspalace/tusky/components/occurrence/OccurrenceActivity.kt
Show resolved
Hide resolved
import java.text.DateFormat | ||
import javax.inject.Inject | ||
|
||
class OccurrenceActivity : BaseActivity(), Injectable, HasAndroidInjector { |
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.
I'd call it DebugActivity/DebugLogActivity or something like that
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.
Hm, it's not used for debugging.
It should/could have the same name as in the ui (and there "debug" would be somewhat misleading).
It shows (hopefully) significant events in the app life. Therefore I was looking for a synonym of "event".
if (durationMs >= 1000) { | ||
Color.MAGENTA | ||
} else if (durationMs >= 400) { | ||
// TODO these colors are a bit of a problem: e. g. on light mode yellow is hardly visible and green is difficult |
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.
make them resources and use different ones for light and dark mode
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.
Done.
android:layout_marginTop="4dp" | ||
android:visibility="gone" | ||
tools:text="Trace line 1" /> | ||
</LinearLayout> |
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.
I'd always use ConstraintLayout for these kind of layout.
It goes something like this:
- Just need to center or frame something -> FrameLayout
- Just need some items below/next to each other -> LinearLayout
- Everything else -> ContaintLayout because it avoids nesting of other layouts and should be faster
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.
Ok, I will try.
(My reasoning would go something like this: a "line" could be considered a group and thus can be grouped. If for example the top level is a vertical linear layout I would know that its contents are "line-ish" things.)
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.
Using ConstraintLayout now.
Cursed two more hours and still have no idea how ConstraintLayout works (with respect to "layout_weight"). By chance I found that width="0dp" does it...
companion object { | ||
fun reduceTrace(stackTrace: Array<StackTraceElement>): String { | ||
// TODO conditions/transforms here are a bit arbitrary... | ||
// TODO we could maybe record more (full stack?) in the db and only reduce it for display? |
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.
These two are the most interesting / to be discussed todos.
(What exactly to record and what to show.)
callTrace = "" | ||
// callTrace = OccurrenceEntity.reduceTrace(Throwable().stackTrace), | ||
) | ||
// TODO all stack traces here have no hint where they might have originated (always ThreadPool) |
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 should have a solution.
Otherwise we only have "something happened but you don't know where/why".
<color name="view_edits_background_insert">@color/tusky_green</color> | ||
<color name="view_edits_background_delete">@color/tusky_red</color> | ||
|
||
<color name="colorSuccess">@color/tusky_green</color> |
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.
NOTE these colors are not yet (all) optimized for reading contrast.
It would probably also be a good idea to record error log(cat) entries. However I found no possibility to transparently intercept calls to Log.e(). So if we want that it would need a rewrite of logging. |
Fixes #3516
Draft: Needs more work and discussion
Potential sources of events: