Skip to content

Conversation

@sidwarkd
Copy link
Contributor

Checklist

  • Component contains License
  • Component contains README.md
  • Component contains idf_component.yml file with url field defined
  • Component was added to upload job
  • Component was added to build job
  • Optional: Component contains unit tests
  • CI passing

Change description

When writing the prompt, the length was incorrectly always the size of a char* pointer. The change uses strlen to properly write the correct size of the provided prompt string.

When writing the prompt, the length was incorrectly always the size of a char* pointer.
@CLAassistant
Copy link

CLAassistant commented Oct 29, 2025

CLA assistant check
All committers have signed the CLA.

@sidwarkd
Copy link
Contributor Author

@SoucheSouche Here is an issue I found with the new component. Otherwise it seems to be working very well.

@github-actions github-actions bot changed the title Fix prompt length issue Fix prompt length issue (IEC-405) Oct 29, 2025
@SoucheSouche
Copy link
Collaborator

@SoucheSouche Here is an issue I found with the new component. Otherwise it seems to be working very well.

@sidwarkd, nice catch! thanks for taking the time to check it out :)
I opened a PR myself with a few fixes here and there and new getter functions (see #597)

@suda-morris suda-morris requested a review from Copilot October 30, 2025 08:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug in the esp_linenoise_dumb function where the prompt length was incorrectly calculated using sizeof() instead of strlen().

  • Corrects the prompt length calculation when writing the prompt in dumb terminal mode
  • Changes from sizeof(config->prompt) to strlen(config->prompt) to properly calculate string length instead of pointer size

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@suda-morris suda-morris enabled auto-merge October 30, 2025 08:28
@suda-morris suda-morris merged commit 75da889 into espressif:master Oct 30, 2025
54 checks passed
@sidwarkd sidwarkd deleted the patch-1 branch October 30, 2025 16:28
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.

5 participants