Skip to content

fix(esp_linenoise): correct history index logic to prevent overwritin… (IEC-527)#732

Open
jjsch-dev wants to merge 1 commit intoespressif:masterfrom
jjsch-dev:fix/esp-linenoise-history-index
Open

fix(esp_linenoise): correct history index logic to prevent overwritin… (IEC-527)#732
jjsch-dev wants to merge 1 commit intoespressif:masterfrom
jjsch-dev:fix/esp-linenoise-history-index

Conversation

@jjsch-dev
Copy link
Copy Markdown

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.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 18, 2026

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot changed the title fix(esp_linenoise): correct history index logic to prevent overwritin… fix(esp_linenoise): correct history index logic to prevent overwritin… (IEC-527) Apr 18, 2026
@jjsch-dev
Copy link
Copy Markdown
Author

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!

@jjsch-dev
Copy link
Copy Markdown
Author

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!

@jjsch-dev jjsch-dev force-pushed the fix/esp-linenoise-history-index branch from c97451d to 96e2026 Compare May 1, 2026 01:23
@jjsch-dev jjsch-dev force-pushed the fix/esp-linenoise-history-index branch from 96e2026 to d9f87cd Compare May 6, 2026 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants