[docs][refactor] update README/CHANGELOG for maxLogcatEntries + refactor LogcatDataSource

[Manabu-GT]

Summary by CodeRabbit

• New Features

  • Added optional thermal status row to compact overlay (Android 11+).
  • Added configurable logcat buffer size to control log history and snapshot replay.

• Documentation

  • Updated changelog with new capabilities.
  • Added configuration guide for customizing logcat buffer limits.

[coderabbitai]

Warning

Rate limit exceeded

@Manabu-GT has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 47 minutes and 14 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fc8a63d4-f100-454a-8c1b-d5600b2ae4f3

📥 Commits

Reviewing files that changed from the base of the PR and between 53fe6a6 and 0057974.

📒 Files selected for processing (1)

• debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt

📝 Walkthrough

Walkthrough

This PR introduces a configurable Logcat buffer size feature, allowing users to control the maximum number of log entries retained via Config.maxLogcatEntries (default 300). The limit is enforced in both continuous panel streaming and bug-report snapshot capture by refactoring LogcatDataSource to use ProcessBuilder instead of Runtime.exec.

Changes

Configurable Logcat Buffer Size

Layer / File(s)	Summary
Feature documentation CHANGELOG.md, README.md	CHANGELOG entry documents the new configurable Logcat buffer size controlled by Config.maxLogcatEntries (default 300), runtime configurability via DebugOverlay.configure, and its effect on log replay. README adds a dedicated section explaining the buffer configuration and its scope on panel opening and snapshot capture.
LogcatDataSource buffer limit implementation debugoverlay-core/src/main/kotlin/com/ms/square/debugoverlay/internal/data/source/LogcatDataSource.kt	Constructor parameter initialMaxEntries is added with @IntRange(from = 1) annotation. Streaming logcat producer subprocess is migrated from Runtime.exec to ProcessBuilder, building explicit logcat arguments with -T <maxEntries>. One-shot snapshot capture subprocess is similarly migrated to ProcessBuilder with -t <maxEntries>, both preserving existing lifecycle management and exception handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• Manabu-GT/DebugOverlay-Android#247: Introduces and wires the maxLogcatEntries config parameter into LogcatDataSource initialization and logcat command construction.
• Manabu-GT/DebugOverlay-Android#126: Modifies the one-shot logcat snapshot path and bug-report behavior that is now bounded by the configurable buffer limit.

Suggested labels

enhancement

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the pull request's main changes: documentation updates (README/CHANGELOG) and a refactor of LogcatDataSource implementation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch followup/logcatdatasource

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude finished @Manabu-GT's task in 2m 0s —— View job

---

Claude Code Review

• Read changed files (CHANGELOG.md, README.md, LogcatDataSource.kt)
• Inspect surrounding context (DebugOverlay.Config, DebugOverlayDataRepository, existing tests)
• Analyze for bugs, quality, security, and performance
• Post review feedback

Summary

A clean docs + small refactor PR. Two substantive items in LogcatDataSource.kt:

1. Add @IntRange(from = 1) to the initialMaxEntries constructor param so the static-analysis contract matches the existing maxEntries property setter and the public Config.maxLogcatEntries validation in DebugOverlay.kt:239-244.
2. Swap Runtime.getRuntime().exec(String) for ProcessBuilder with an explicit argument list, in both the streaming producer and the one-shot snapshot capture.

Looks good overall. Behavior is preserved (no -t/-T semantics change), and the docs additions in README.md:194-204 and CHANGELOG.md accurately reflect the existing feature.

Strengths

• ProcessBuilder over Runtime.exec(String) is the right call. exec(String) tokenizes on whitespace via a legacy StringTokenizer, which is brittle and historically a vector for argument-handling bugs. The new explicit argument list removes any ambiguity even though maxEntries is an Int and the inputs were already safe. (Bonus: matches Java's own recommendation since 17 that Runtime.exec(String) is discouraged.)
• @IntRange(from = 1) on the ctor param is consistent with the property declaration at LogcatDataSource.kt:62-66 and the public-API runtime check at DebugOverlay.kt:241-244. Good defense-in-depth: runtime validation at the public surface, static lint inside.
• Docs are accurate and concise. README.md:204 correctly notes the value bounds both -T N (streaming, panel-open) and -t N (snapshot, bug report). CHANGELOG entry calls out the runtime reassignment caveat that's already documented in maxEntries' KDoc.

Issues / Suggestions

1. nit — stderr is left unconsumed (pre-existing, made trivial to fix by the refactor)

LogcatDataSource.kt:96-106 and :176-182: neither ProcessBuilder invocation consumes the subprocess' stderr. With logcat this is harmless in practice (it produces ~no stderr), but the general contract of Process is that any unread stream can deadlock the child if its pipe buffer fills. The ProcessBuilder refactor makes the fix a one-liner — see inline suggestion — but feel free to defer; this is a latent issue that the PR didn't introduce.

2. nit — Stale typo in surrounding KDoc

LogcatDataSource.kt:94: "-t option fetches once and exists immediately" — exists → exits. Pre-existing typo; PR diff doesn't touch the line, so this is just an FYI if you want to fold in a one-character fix. Fix this →

3. question — No test coverage for the LogcatDataSource refactor

The behavior change (process invocation mechanism) is pure refactor, and DebugOverlayTest.kt:30-69 covers Config.maxLogcatEntries validation at the public surface. That feels sufficient given (a) Runtime.exec vs ProcessBuilder is an equivalence transformation here, and (b) the inner process call is hard to unit-test without launching logcat. No action needed — flagging only so it's an explicit decision rather than an omission.

Security / Performance

• Security: Argument injection was never reachable (maxEntries: Int → Int.toString()), but switching off exec(String) is still a good defensive move.
• Performance: No expected change. Same process, same flags, same pipes.
  · Branch: followup/logcatdatasource

[claude]

Nit (pre-existing, surfaced by the refactor): ProcessBuilder (like Runtime.exec) leaves stderr as a separate pipe that nothing consumes. With logcat this is harmless in practice — stderr is essentially silent — but if the OS pipe buffer ever filled the producer would deadlock. Now that the refactor uses ProcessBuilder, it's a one-liner to bulletproof it: …).redirectErrorStream(true).start(). Same for captureLogcatOnce() at line 182. Optional — feel free to defer.

[github-actions]

Qodana for JVM

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at qodana-support@jetbrains.com

• Or via our issue tracker: https://jb.gg/qodana-issue
• Or share your feedback: https://jb.gg/qodana-discussions