Provide toggle to turn ON/OFF debug log#98
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsFragment
participant SettingsViewModel
participant MemoPreferences
participant ALog
User->>SettingsFragment: Toggle "Enable debug log" switch
SettingsFragment->>SettingsViewModel: setEnableDebugLog(isChecked)
SettingsViewModel->>MemoPreferences: setEnableDebugLog(isChecked)
SettingsFragment->>ALog: Set isEnableLog = isChecked
sequenceDiagram
participant App
participant MemoPreferences
participant ALog
App->>MemoPreferences: isEnableDebugLog()
MemoPreferences-->>App: enabled (Boolean)
App->>ALog: init(isEnabled = enabled)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This PR must be merged after #96 |
tuancoltech
force-pushed
the
feat/toggle-on-off-debug-log
branch
from
48727c7 to
3813b5b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (3)
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/GraphicNotesViewModel.kt (1)
46-53: Potential NPE when note.svg is null
note.svg?.getPaths()!!will throw ifnote.svgisnull.
Either require non-null before entering the coroutine or handle the null case gracefully:- if (editPaths.isNotEmpty()) editPaths.clear() - editPaths.addAll(note.svg?.getPaths()!!) - svg = note.svg.copy() + if (editPaths.isNotEmpty()) editPaths.clear() + note.svg?.let { + editPaths.addAll(it.getPaths()) + svg = it.copy() + } ?: ALog.w(TAG, "Graphic note opened without SVG, ignoring")app/src/main/java/dev/arkbuilders/arkmemo/utils/ContextExt.kt (1)
20-34: Elvis branch never executedThe block:
activityLauncher?.let { activityLauncher.launch(settingIntent) } ?: { this.startActivity(settingIntent) }returns a lambda instead of executing
startActivity()whenactivityLauncheris null.
This existed before the logging change but is still a latent bug.Replace with a simple
if/else:-activityLauncher?.let { - activityLauncher.launch(settingIntent) -} ?: { - this.startActivity(settingIntent) -} +if (activityLauncher != null) { + activityLauncher.launch(settingIntent) +} else { + startActivity(settingIntent) +}app/src/main/java/dev/arkbuilders/arkmemo/repo/text/TextNotesRepo.kt (1)
60-86: Temp file leaks when a duplicate note is detectedIf a note with identical content already exists, the early
return@withContextpath leaves the temporary file created bycreateTempFile()undeleted. Over time this can accumulate orphan files.- return@withContext + kotlin.io.path.deleteIfExists(tempPath) + return@withContext(Consider wrapping temp-file usage in
try { … } finally { deleteIfExists(tempPath) }for full safety.)
🧹 Nitpick comments (18)
logging/src/main/AndroidManifest.xml (1)
1-2: Consider omitting thepackageattributeIn AGP 8+ the recommended place for a library’s namespace is the
namespacefield inbuild.gradle. Keeping it only in Gradle avoids duplication and Lint warnings.Optional cleanup:
-<manifest package="dev.arkbuilders.logging"/> +<manifest/>app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (1)
46-46: Log the throwable as well to keep stack-traceOnly the exception message is logged; the stack-trace is lost, making debugging harder.
-ALog.e(TAG, "pasteNoteClickListener exception: ${e.message}") +ALog.e(TAG, "pasteNoteClickListener exception", e)Requires
ALog.e(tag: String, msg: String, tr: Throwable)overload (same asandroid.util.Log.e).app/src/main/java/dev/arkbuilders/arkmemo/ui/views/WaveView.kt (1)
50-52: Avoid eager string interpolation in hot path
computeWave()is called for every visual update; the log message is interpolated on every invocation even when debug logging is OFF, causing unnecessary allocations.Consider guarding the call or offering a lazy API to
ALog:- ALog.d(TAG, "computeWave amplitude: $amplitude height: $height") + if (ALog.isLoggable(ALog.DEBUG)) { + ALog.d(TAG, "computeWave amplitude: $amplitude height: $height") + }(or add a
ALog.d(tag) { "msg" }overload inside the logging module).app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkMediaPlayerFragment.kt (1)
102-104: Guard high-frequency log to avoid overhead
handleSideEffectcan be triggered many times per second during playback.
Wrap the call or add a lazy logger to skip building the string when debug is disabled.app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.kt (1)
318-320: Same eager-interpolation concern as elsewhereThe log inside
handlePlaySideEffect()executes on every side-effect; add a debug guard to avoid needless string building when logging is off.app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkMediaPlayerViewModel.kt (2)
79-82: Guard debug traces inside tight loops
setupVisualizer()is called once per playback start, so the debug trace is harmless.
Nevertheless, keep in mind that audio initialisation happens on the main thread; excessive logging here may become visible on low-end devices.
121-123: Heavy-frequency logging may hurt performance
computeFftMagnitude()can be executed hundreds of times per second. Even when logging is disabled, string concatenation still incurs minor cost.If the log is just for occasional troubleshooting, wrap it with
if (ALog.isLoggable())(or similar helper) or remove it.app/src/main/java/dev/arkbuilders/arkmemo/utils/ContextExt.kt (2)
16-17: Minor: avoid string concatenation in disabled logsBoth calls build the string even when logging is off. Prefer:
ALog.e("openLink", e) // overload that formats internallyor
if (ALog.isLoggable()) ALog.e("openLink", "exception: ${e.message}")
33-34: Same note as aboveConsider using an overload that accepts
Throwabledirectly to preserve stack-trace and avoid manual message building.app/src/main/java/dev/arkbuilders/arkmemo/repo/voices/VoiceNotesRepo.kt (1)
79-82: Consider lowering log levelThe “resource already exists” message may be more suited for
ALog.vorALog.idepending on frequency.app/src/main/res/drawable/ic_debug_log.xml (1)
7-8: Use theme-aware color instead of hard-coded blackHard-coding
#FF000000will make the icon always pure black, which may be invisible on dark backgrounds or clash with theming. Prefer a theme/color resource so the icon adapts automatically.- android:fillColor="#FF000000" + android:fillColor="?attr/colorOnSurface" <!-- or @color/whatever -->app/src/main/java/dev/arkbuilders/arkmemo/repo/text/TextNotesRepo.kt (1)
127-134: Exception swallowed — at least propagate or wrap
readStorageconverts any I/O failure into an empty note that is immediately filtered out, silently hiding the problem from callers. Forward the exception (or wrap it in a domain error) so UI or telemetry can react appropriately.logging/src/test/java/dev/arkbuilders/logging/ExampleUnitTest.kt (1)
12-16: Duplicate “2 + 2 == 4” test adds no valueAn identical placeholder test already exists in
app/src/test/.../ExampleUnitTest.kt. Either remove this copy or replace it with tests that actually verifyALogenable/disable behaviour.logging/proguard-rules.pro (1)
1-21: Standard ProGuard template - consider logging-specific rules.The template ProGuard configuration is appropriate for a new Android library module. Consider whether any specific rules are needed for the ALog implementation, particularly if reflection or dynamic method calls are used in the logging module.
logging/src/androidTest/java/dev/arkbuilders/logging/ExampleInstrumentedTest.kt (1)
18-23: Consider adding tests for logging functionality.While this basic context test is functional, consider adding instrumented tests that verify the actual ALog functionality, such as:
- ALog initialization behavior
- Enable/disable logging functionality
- Proper delegation to Android's Log system
logging/src/main/java/dev/arkbuilders/logging/ALog.kt (1)
10-12: Consider adding parameter validation.While the current implementation works, consider adding basic validation to make the API more robust.
fun init(isEnabled: Boolean = false) { + // Optional: Add logging to track initialization state + // android.util.Log.d("ALog", "Initializing logging with enabled=$isEnabled") isEnableLog = isEnabled }logging/src/main/java/dev/arkbuilders/logging/IArkLog.kt (1)
1-10: Well-designed logging interface.The
IArkLoginterface effectively mirrors Android'sLogclass API, which provides:
- Familiar method signatures for developers
- Consistent return types (
Int)- Flexible nullable
tagparameters- Two
e()method variants for error logging with/withoutThrowableThis design ensures easy migration from Android's
Logto the custom logging system.Consider adding KDoc documentation to describe the interface purpose and method parameters:
+/** + * Interface for ARK logging functionality, providing methods for different log levels. + */ interface IArkLog { + /** Log a debug message */ fun d(tag: String?, msg: String): Int + /** Log an info message */ fun i(tag: String?, msg: String): Int // ... etc for other methodsapp/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/SettingsViewModel.kt (1)
25-33: Functional implementation with minor naming inconsistency.The debug log methods correctly follow the established patterns:
setEnableDebugLog()usesviewModelScope.launchfor async preference storageisEnableDebugLog()provides synchronous access to the preferenceConsider renaming for consistency with existing methods:
setEnableDebugLog()→storeEnableDebugLog()isEnableDebugLog()→getEnableDebugLog()This would match the existing
storeCrashReportEnabled()/getCrashReportEnabled()pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (42)
app/build.gradle(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/App.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/graphics/SVG.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/media/ArkAudioRecorderImpl.kt(5 hunks)app/src/main/java/dev/arkbuilders/arkmemo/media/ArkMediaPlayerImpl.kt(4 hunks)app/src/main/java/dev/arkbuilders/arkmemo/preferences/MemoPreferences.kt(1 hunks)app/src/main/java/dev/arkbuilders/arkmemo/preferences/MemoPreferencesImpl.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/NotesRepoHelper.kt(6 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/graphics/GraphicNotesRepo.kt(6 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/text/TextNotesRepo.kt(5 hunks)app/src/main/java/dev/arkbuilders/arkmemo/repo/voices/VoiceNotesRepo.kt(4 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.kt(4 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkMediaPlayerFragment.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt(3 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/SettingsFragment.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkMediaPlayerViewModel.kt(4 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkRecorderViewModel.kt(6 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/GraphicNotesViewModel.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/NotesViewModel.kt(6 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/QRViewModel.kt(3 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/SettingsViewModel.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/ui/views/WaveView.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/utils/ContextExt.kt(2 hunks)app/src/main/java/dev/arkbuilders/arkmemo/utils/Utils.kt(2 hunks)app/src/main/res/drawable/ic_debug_log.xml(1 hunks)app/src/main/res/layout/fragment_settings.xml(1 hunks)app/src/main/res/values/strings.xml(1 hunks)build.gradle(1 hunks)gradle/libs.versions.toml(1 hunks)logging/.gitignore(1 hunks)logging/build.gradle(1 hunks)logging/proguard-rules.pro(1 hunks)logging/src/androidTest/java/dev/arkbuilders/logging/ExampleInstrumentedTest.kt(1 hunks)logging/src/main/AndroidManifest.xml(1 hunks)logging/src/main/java/dev/arkbuilders/logging/ALog.kt(1 hunks)logging/src/main/java/dev/arkbuilders/logging/ArkLogImpl.kt(1 hunks)logging/src/main/java/dev/arkbuilders/logging/IArkLog.kt(1 hunks)logging/src/test/java/dev/arkbuilders/logging/ExampleUnitTest.kt(1 hunks)settings.gradle(1 hunks)
🧰 Additional context used
🧠 Learnings (20)
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/QRViewModel.kt (1)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
app/src/main/java/dev/arkbuilders/arkmemo/graphics/SVG.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/NotesViewModel.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
app/src/main/java/dev/arkbuilders/arkmemo/utils/Utils.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/GraphicNotesViewModel.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
app/src/main/java/dev/arkbuilders/arkmemo/repo/voices/VoiceNotesRepo.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkRecorderFragment.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/ArkMediaPlayerFragment.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
app/src/main/java/dev/arkbuilders/arkmemo/repo/text/TextNotesRepo.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
app/src/main/java/dev/arkbuilders/arkmemo/repo/NotesRepoHelper.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
app/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
app/src/main/java/dev/arkbuilders/arkmemo/repo/graphics/GraphicNotesRepo.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkMediaPlayerViewModel.kt (1)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/SettingsFragment.kt (1)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
app/src/main/java/dev/arkbuilders/arkmemo/App.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/SettingsViewModel.kt (1)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkRecorderViewModel.kt (2)
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:40:19.372Z
Learning: In the ARK-Memo Android app, the notesViewModel.init {} callback properly handles threading by dispatching operations to worker threads, preventing main thread blocking during view restoration.
Learnt from: tuancoltech
PR: ARK-Builders/ARK-Memo#0
File: :0-0
Timestamp: 2025-07-18T05:33:52.382Z
Learning: In the ARK-Memo Android app, TextNotesRepo.findNote() intentionally returns null because text notes don't need special handling during view restoration. This is a design decision to ignore text notes when restoring view state, as users can easily continue editing text notes without complex state recovery mechanisms.
🧬 Code Graph Analysis (2)
app/src/main/java/dev/arkbuilders/arkmemo/repo/voices/VoiceNotesRepo.kt (1)
app/src/main/java/dev/arkbuilders/arkmemo/repo/NotesRepo.kt (1)
function(8-11)
app/src/main/java/dev/arkbuilders/arkmemo/repo/graphics/GraphicNotesRepo.kt (1)
app/src/main/java/dev/arkbuilders/arkmemo/repo/NotesRepo.kt (1)
function(8-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ktlint
- GitHub Check: lint
🔇 Additional comments (54)
gradle/libs.versions.toml (1)
69-70: Define usage or drop the new aliasThe new
android-libraryplugin alias is well-formed, but no module’sbuild.gradlefile is shown adopting it. If theloggingmodule (or any future module) keeps an explicitid("com.android.library")string instead ofalias(libs.plugins.android.library), the alias will be dead code.Request: verify all library modules are migrated to the alias or remove the alias to avoid silent drift.
logging/.gitignore (1)
1-1: LGTM – ignore build artefactsIgnoring the module’s
/buildfolder is standard and keeps VCS clean.app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditTextNotesFragment.kt (1)
22-22: Dependency switched toALog👍Import replacement aligns the fragment with the new centralised logger.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/NotesFragment.kt (2)
45-45: Import swap looks goodThe fragment now uses the centralised logger; no further action.
319-319: Guard noisy debug log behind flavour/flag if neededWith
ALogdisabled in release this line is harmless; ifisEnableLogcan be toggled at runtime, ensure large lists don’t spam logs when users enable it.No change requested.
settings.gradle (1)
62-64: Logging module correctly registeredIncluding
':logging'here makes the module resolvable by Gradle; no problems spotted.app/src/main/res/values/strings.xml (1)
130-130: LGTM! String resource follows naming convention.The new string resource is properly named, clearly worded, and appropriately placed in the Settings section.
app/src/main/java/dev/arkbuilders/arkmemo/media/ArkMediaPlayerImpl.kt (2)
5-5: LGTM! Proper import update.Import correctly updated to use the new custom logging utility.
27-27: LGTM! Consistent logging replacement.All logging calls have been consistently updated from Android's Log to the new ALog utility while maintaining the same log levels and messages.
Also applies to: 51-51, 57-57, 62-62, 71-71, 76-76
app/src/main/java/dev/arkbuilders/arkmemo/graphics/SVG.kt (2)
8-8: LGTM! Import updated correctly.Import properly changed to use the new custom logging utility.
139-139: LGTM! Logging call updated consistently.The logging call has been properly updated to use ALog while maintaining the same log level and message.
app/src/main/java/dev/arkbuilders/arkmemo/utils/Utils.kt (2)
15-15: LGTM! Import updated properly.Import correctly changed to use the new custom logging utility.
138-138: LGTM! Error logging updated consistently.The error logging call has been properly updated to use ALog while maintaining the same error handling behavior.
app/build.gradle (1)
122-122: Logging module integration verified successfullyAll checks have passed; the logging module is present and properly configured:
- logging/build.gradle exists with correct Android library setup
- ALog class found at logging/src/main/java/dev/arkbuilders/logging/ALog.kt
- settings.gradle includes
':logging'No further action needed—approving the dependency addition.
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/QRViewModel.kt (2)
39-40: Consistent migration – looks good
Log.d→ALog.dis applied correctly and keeps the original tag/message contract.
No further action needed here.
56-57: LGTMSame remark – correct one-to-one replacement.
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkMediaPlayerViewModel.kt (1)
175-177: LGTMProgress-monitor trace migrated correctly.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/EditGraphicNotesFragment.kt (2)
239-242: Good swapDebug trace migrated; no concerns.
275-277: Good swapVerbose trace migrated; no concerns.
app/src/main/java/dev/arkbuilders/arkmemo/repo/voices/VoiceNotesRepo.kt (3)
61-66: LGTM
ALog.dreplacement is correct and placed on the I/O dispatcher – negligible overhead.
73-74: LGTMSame remark.
104-106: LGTMRead-storage debug trace migrated correctly.
app/src/main/java/dev/arkbuilders/arkmemo/repo/graphics/GraphicNotesRepo.kt (2)
22-22: LGTM: Clean import replacement for custom logging.The import has been correctly updated to use the custom
ALogutility from the new logging module.
83-83: LGTM: Consistent logging utility replacement.All logging calls have been correctly migrated from Android's
Logto the customALogutility while maintaining the same logging levels and message formats. This enables centralized control over debug logging behavior.Also applies to: 88-88, 101-101, 113-113, 119-119, 123-123, 152-152, 195-195
build.gradle (1)
4-4: LGTM: Proper Android library plugin configuration.Adding the Android library plugin alias enables submodules to apply it as needed, which supports the new logging module introduced in this PR. The
apply falsepattern is correct for root-level plugin management.app/src/main/java/dev/arkbuilders/arkmemo/App.kt (2)
8-8: LGTM: Clean import addition for custom logging.The import statement correctly references the new custom logging utility.
21-21: LGTM: Proper ALog initialization during app startup.The ALog initialization is correctly placed after essential dependencies and uses the persisted user preference to control debug logging behavior. The explicit parameter naming (
isEnabled) makes the intent clear.app/src/main/java/dev/arkbuilders/arkmemo/repo/NotesRepoHelper.kt (2)
12-12: LGTM: Import correctly updated for custom logging.The import has been properly replaced to use the custom
ALogutility.
50-50: LGTM: Consistent logging migration across all calls.All logging statements have been correctly migrated from Android's
LogtoALogwhile preserving the original logging levels (debug and error) and message content. This maintains consistency with the broader logging system refactoring.Also applies to: 61-61, 94-94, 101-101, 122-122, 131-131, 137-137
app/src/main/res/layout/fragment_settings.xml (1)
24-34: LGTM: Well-structured debug log toggle addition.The new debug log setting is properly integrated into the layout with:
- Consistent design matching the existing crash reporting toggle
- Appropriate default state (enabled but unchecked) for opt-in debug logging
- Proper constraint layout positioning and spacing
- Correct resource references for the new string and icon resources
app/src/main/java/dev/arkbuilders/arkmemo/ui/activities/MainActivity.kt (1)
42-61: ALog integration looks goodAll direct
Log.dusages were replaced withALog.d, keeping a single, toggleable logging backend. No further issues spotted.app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/ArkRecorderViewModel.kt (1)
58-78: ALog migration LGTMLogging calls were migrated cleanly; no functional changes introduced.
app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/NotesViewModel.kt (1)
18-18: LGTM! Consistent logging replacement.The replacement of Android's
Log.dwithALog.dis implemented consistently across all debug logging statements. The import is correctly added and all existing TAG usage is preserved.Also applies to: 52-52, 67-67, 83-83, 117-117, 178-181
app/src/main/java/dev/arkbuilders/arkmemo/media/ArkAudioRecorderImpl.kt (1)
7-7: LGTM! Complete logging system migration.The replacement of both
Log.dandLog.ewith theirALogequivalents is implemented consistently throughout the class. Both debug and error logging are properly migrated to the new centralized logging system.Also applies to: 39-39, 44-44, 49-49, 54-54, 63-63, 75-75, 87-87
app/src/main/java/dev/arkbuilders/arkmemo/preferences/MemoPreferences.kt (1)
22-24: LGTM! Clean interface extension for debug log preference.The new methods follow the established naming conventions and provide a clear contract for managing debug log preferences. The method signatures are appropriate for boolean preference storage and retrieval.
app/src/main/java/dev/arkbuilders/arkmemo/ui/fragments/SettingsFragment.kt (4)
15-15: LGTM!The import for the new logging module is correctly added to support the debug log toggle functionality.
47-51: LGTM! Consistent implementation pattern.The debug log toggle follows the same pattern as the existing crash report toggle, providing immediate UI feedback by updating the global
ALog.isEnableLogflag while persisting the preference asynchronously via the ViewModel.
15-15: LGTM! Clean import addition.The import for
ALogfrom the new logging module is properly added and follows the existing import organization.
47-51: Well-structured debug log toggle implementation.The debug log toggle follows the exact same pattern as the existing crash report toggle, maintaining consistency. The implementation correctly:
- Sets initial state from ViewModel's
isEnableDebugLog()- Updates
ALog.isEnableLogimmediately for real-time logging control- Persists the preference via
settingsViewModel.setEnableDebugLog()This ensures both immediate UI feedback and proper persistence.
app/src/main/java/dev/arkbuilders/arkmemo/preferences/MemoPreferencesImpl.kt (4)
15-15: LGTM!The preference key constant follows the established naming convention and is appropriately defined.
52-58: LGTM!The debug log preference methods are correctly implemented following the established pattern, with an appropriate default value of
falsefor debug logging.
15-15: Good constant naming and placement.The
PREF_ENABLE_DEBUG_LOGconstant follows the established naming convention and is appropriately placed with other preference constants.
52-58: Consistent preference implementation.The debug log preference methods follow the exact same pattern as existing preferences:
setEnableDebugLog()usesprefEditor.putBoolean().apply()for async persistenceisEnableDebugLog()returns the stored value with a sensible default offalseThe default value of
falseis appropriate for debug logging (opt-in behavior).logging/build.gradle (3)
1-4: LGTM!The plugin configuration correctly uses aliases for Android library and Kotlin Android plugins, following modern Gradle practices.
6-28: LGTM!The Android configuration is appropriate for a logging library with reasonable SDK versions (compileSdk 34, minSdk 26) and Java/Kotlin version 11. Disabling minification for release builds is sensible for a logging utility.
30-38: LGTM!The dependencies are appropriate for an Android logging library, including standard Android libraries and comprehensive test dependencies.
logging/src/main/java/dev/arkbuilders/logging/IArkLog.kt (1)
3-10: LGTM! Well-designed logging interface.The interface correctly mirrors Android's Log API with appropriate method signatures and return types. The nullable
msgparameter in the error method with Throwable (line 8) differs from other methods - this appears intentional to match Android Log's signature where the message can be null when a Throwable is provided.app/src/main/java/dev/arkbuilders/arkmemo/ui/viewmodels/SettingsViewModel.kt (3)
13-14: LGTM!The constructor formatting change improves code conciseness without affecting functionality.
25-33: LGTM! Consistent implementation pattern.The debug log preference methods correctly follow the established pattern - asynchronous storage using
viewModelScopeand synchronous retrieval, matching the existing crash report preference methods.
13-14: LGTM! Constructor formatting improvement.The single-line constructor format is cleaner and more readable.
logging/src/main/java/dev/arkbuilders/logging/ArkLogImpl.kt (1)
5-30: LGTM! Clean wrapper implementation.The
ArkLogImplclass provides a clean, direct wrapper around Android'sLogclass. The implementation correctly:
- Delegates all calls to the corresponding Android Log methods
- Preserves return values for API compatibility
- Covers all standard log levels (debug, info, verbose, warning, error)
- Provides proper error method overloads
logging/src/main/java/dev/arkbuilders/logging/ALog.kt (3)
5-5: Good use of lazy initialization.The lazy delegate ensures thread-safe initialization of the
ArkLogImplinstance only when needed.
7-8: Proper thread safety with volatile flag.The
@Volatileannotation ensures that changes toisEnableLogare immediately visible across all threads, which is essential for a global logging toggle.
14-42: Efficient early return pattern with consistent API.The implementation correctly:
- Checks the enable flag before any logging work
- Returns a consistent -1 value when disabled
- Maintains the same method signatures as Android Log
- Covers all standard log levels with proper overloads
This provides an efficient toggle mechanism while preserving API compatibility.
2 participants
Description
Provide toggle to turn ON/OFF debug log
Ticket
https://app.asana.com/1/1207783906637200/project/1207819682524134/task/1208779829497391
Video
Screen.Recording.2025-07-30.at.21.23.30.mov
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores