fix(esp_linenoise): correct history index logic to prevent overwritin… (IEC-527)#732
fix(esp_linenoise): correct history index logic to prevent overwritin… (IEC-527)#732jjsch-dev wants to merge 1 commit intoespressif:masterfrom
Conversation
|
Hi team! Just a gentle ping on this PR. We are currently finalizing the porting of the Barracuda App Server (Xedge32) to support the new ESP32-P4 and ESP32-S3 architectures on IDF v6.1. This specific fix in esp_linenoise is critical for our console stability and is currently a blocker for our upcoming release. You can check out the project here: Xedge32 Please let me know if there's anything else you need from my side to help get this merged. Thanks for your time! |
|
Hi team, just a quick ping on this PR. The branch has been merged with the latest master to ensure there are no conflicts. Since it addresses an active bug with the history index overriding the current input, it would be great to get it reviewed. Let me know if you need any changes on my end! |
c97451d to
96e2026
Compare
96e2026 to
d9f87cd
Compare
Description / Motivation:
This PR fixes a critical logic bug in the esp_linenoise component where pressing the UP arrow key overwrites a valid history command with an empty string, effectively breaking history navigation and resulting in an empty prompt.
Root Cause Analysis:
In esp_linenoise_edit(), the engine appends an empty string "" to act as a temporary scratchpad for the current line. However, the history index is hardcoded to zero (state->history_index = 0;).
Because the current implementation appends new commands to the end of the history array (rather than shifting them to the beginning), index 0 points to the oldest command in the array, not the newly added temporary scratchpad.
When the user presses the UP arrow (esp_LINENOISE_HISTORY_PREV):
The engine saves the current empty buffer into history[history_index] (which is history[0]), silently destroying the valid command stored there.
The navigation math (state->history_index += (dir == esp_LINENOISE_HISTORY_PREV) ? 1 : -1;) assumes the old reverse-order logic, moving the index in the wrong direction.
The Fix:
Index Initialization: Updated esp_linenoise_edit() to point the index to the end of the array (the actual scratchpad):
state->history_index = state->history_length - 1;
Navigation Math: Inverted the direction logic in esp_linenoise_edit_history_next() to properly navigate backward for older commands and forward for newer ones:
state->history_index += (dir == esp_LINENOISE_HISTORY_PREV) ? -1 : 1;
This restores the expected behavior of the arrow keys and protects the integrity of the history array.