Move replace_token to component as replace_all#60
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded an in-place string helper Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/nspanel_easy/text.cpp`:
- Around line 181-189: The replace_all function can infinite-loop if token is an
empty string; add a guard at the start of replace_all(std::string& str, const
char* token, const char* value) to return immediately (or no-op) when token_len
== 0 to avoid calling str.find("") repeatedly; ensure the early-return occurs
before computing/searching and keep the existing replacement logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0fdec0d6-b191-4b86-ab1d-92b23809b3d1
📒 Files selected for processing (3)
components/nspanel_easy/text.cppcomponents/nspanel_easy/text.hesphome/nspanel_esphome_datetime.yaml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/validate_clang_format.yml:
- Line 23: The workflow uses github.head_ref alone (ref: ${{ github.head_ref
}}), which is null for workflow_dispatch runs; change that ref to use a fallback
expression such as ref: ${{ github.head_ref || github.ref }} so checkout targets
the PR branch when present and falls back to the workflow ref (or SHA) for
workflow_dispatch; update the checkout step that currently references
github.head_ref to use this fallback expression.
- Around line 30-32: The find | xargs pipeline is fragile to filenames with
spaces/newlines and to empty input; update the invocation that uses find, xargs,
and clang-format so find emits null-delimited results (use find ... -print0) and
xargs consumes them safely with the null flag and the no-op-if-empty flag (use
xargs -0 --no-run-if-empty or equivalent), passing the file list into
clang-format --style=file -i; change the pipeline that currently uses find ... |
xargs clang-format to use these flags to ensure robustness.
- Around line 34-39: Guard the "Commit clang-format fixes" auto-commit step
(uses: stefanzweifel/git-auto-commit-action@v6) so it only runs for same-repo
pull requests or workflow_dispatch runs by adding a proper if expression with
the ${{ }} delimiters; for example use if: ${{ github.event_name ==
'workflow_dispatch' || (github.event_name == 'pull_request' &&
github.event.pull_request.head.repo.full_name == github.repository) }} to
prevent attempts to push from forked PRs where GITHUB_TOKEN is read-only.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa25443d-5ef0-4eb5-9033-e26c4df6c279
📒 Files selected for processing (2)
.clang-format.github/workflows/validate_clang_format.yml
✅ Files skipped from review due to trivial changes (1)
- .clang-format
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.test/unit/test_addon_climate.cpp (1)
6-8: Add explicit<set>include to avoid transitive-include fragility.This file uses
std::setbut does not include<set>directly. It currently relies on indirect headers, which is brittle across toolchain/header changes.Suggested diff
`#include` <gtest/gtest.h> `#include` <string> +#include <set>Also applies to: 94-96, 119-121, 324-337
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.test/unit/test_addon_climate.cpp around lines 6 - 8, This test file and several other ranges use std::set but rely on transitive includes; add an explicit `#include` <set> at the top of .test/unit/test_addon_climate.cpp (near the existing `#include` <string> / `#include` <gtest/gtest.h>) and similarly add explicit <set> includes where referenced in the other noted ranges (lines 94-96, 119-121, 324-337) so the tests no longer break if indirect headers change; search for uses of std::set in the file to ensure every translation unit that uses it includes <set> directly..test/unit/test_base.cpp (1)
8-10: Include<cstdint>explicitly foruint32_t.
delay(uint32_t ms)currently relies on transitive includes for fixed-width integer types. Add<cstdint>directly in this file for robust compilation.Suggested diff
`#include` <gtest/gtest.h> `#include` <gmock/gmock.h> +#include <cstdint> `#include` <string> `#include` <map>Also applies to: 29-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.test/unit/test_base.cpp around lines 8 - 10, Add an explicit include for the fixed-width integer header so uint32_t is defined: add `#include` <cstdint> to the top of this test file (and any other test files where delay(uint32_t ms) is declared/used) to avoid relying on transitive includes; search for usages of delay(uint32_t) or references to uint32_t and ensure those files include <cstdint>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.test/unit/test_addon_climate.cpp:
- Around line 6-8: This test file and several other ranges use std::set but rely
on transitive includes; add an explicit `#include` <set> at the top of
.test/unit/test_addon_climate.cpp (near the existing `#include` <string> /
`#include` <gtest/gtest.h>) and similarly add explicit <set> includes where
referenced in the other noted ranges (lines 94-96, 119-121, 324-337) so the
tests no longer break if indirect headers change; search for uses of std::set in
the file to ensure every translation unit that uses it includes <set> directly.
In @.test/unit/test_base.cpp:
- Around line 8-10: Add an explicit include for the fixed-width integer header
so uint32_t is defined: add `#include` <cstdint> to the top of this test file (and
any other test files where delay(uint32_t ms) is declared/used) to avoid relying
on transitive includes; search for usages of delay(uint32_t) or references to
uint32_t and ensure those files include <cstdint>.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 096b9531-00d5-4514-ad21-76942ba5807c
📒 Files selected for processing (40)
.test/unit/test_addon_climate.cpp.test/unit/test_addon_upload_tft.cpp.test/unit/test_base.cppcomponents/nspanel_easy/addon_climate.cppcomponents/nspanel_easy/addon_climate.hcomponents/nspanel_easy/addon_upload_tft.cppcomponents/nspanel_easy/addon_upload_tft.hcomponents/nspanel_easy/base.cppcomponents/nspanel_easy/base.hcomponents/nspanel_easy/ha_components.cppcomponents/nspanel_easy/ha_components.hcomponents/nspanel_easy/hardware.hcomponents/nspanel_easy/hw_display.cppcomponents/nspanel_easy/hw_display.hcomponents/nspanel_easy/icons.hcomponents/nspanel_easy/macros.hcomponents/nspanel_easy/nextion_components.cppcomponents/nspanel_easy/nextion_components.hcomponents/nspanel_easy/nspanel_easy.hcomponents/nspanel_easy/page_alarm.hcomponents/nspanel_easy/page_climate.cppcomponents/nspanel_easy/page_climate.hcomponents/nspanel_easy/page_entities.cppcomponents/nspanel_easy/page_entities.hcomponents/nspanel_easy/page_home.hcomponents/nspanel_easy/page_media_player.cppcomponents/nspanel_easy/page_media_player.hcomponents/nspanel_easy/page_notification.cppcomponents/nspanel_easy/page_notification.hcomponents/nspanel_easy/page_qrcode.hcomponents/nspanel_easy/page_utilities.cppcomponents/nspanel_easy/page_utilities.hcomponents/nspanel_easy/pages.cppcomponents/nspanel_easy/pages.hcomponents/nspanel_easy/text.cppcomponents/nspanel_easy/text.hcomponents/nspanel_easy/versioning.cppcomponents/nspanel_easy/versioning.hcomponents/nspanel_easy/weather.cppcomponents/nspanel_easy/weather.h
✅ Files skipped from review due to trivial changes (37)
- components/nspanel_easy/icons.h
- components/nspanel_easy/page_entities.h
- components/nspanel_easy/page_media_player.h
- components/nspanel_easy/addon_climate.cpp
- components/nspanel_easy/addon_upload_tft.h
- components/nspanel_easy/pages.cpp
- components/nspanel_easy/versioning.cpp
- components/nspanel_easy/page_climate.cpp
- components/nspanel_easy/hw_display.h
- components/nspanel_easy/nspanel_easy.h
- components/nspanel_easy/page_notification.h
- components/nspanel_easy/page_notification.cpp
- components/nspanel_easy/page_entities.cpp
- components/nspanel_easy/ha_components.cpp
- components/nspanel_easy/hw_display.cpp
- components/nspanel_easy/page_media_player.cpp
- components/nspanel_easy/weather.cpp
- components/nspanel_easy/page_alarm.h
- components/nspanel_easy/addon_upload_tft.cpp
- components/nspanel_easy/addon_climate.h
- components/nspanel_easy/base.h
- components/nspanel_easy/page_qrcode.h
- components/nspanel_easy/nextion_components.cpp
- components/nspanel_easy/macros.h
- components/nspanel_easy/base.cpp
- components/nspanel_easy/ha_components.h
- components/nspanel_easy/text.cpp
- components/nspanel_easy/page_climate.h
- components/nspanel_easy/hardware.h
- components/nspanel_easy/versioning.h
- components/nspanel_easy/page_utilities.cpp
- components/nspanel_easy/weather.h
- components/nspanel_easy/page_utilities.h
- components/nspanel_easy/nextion_components.h
- components/nspanel_easy/pages.h
- components/nspanel_easy/page_home.h
- components/nspanel_easy/text.h
…into v9999.99.9
…into v9999.99.9
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Refactor
Bug Fixes
Chores