Skip to content

feat(linenoise): Add linenoise library (IEC-328) #526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

feat(linenoise): Add linenoise library (IEC-328) #526

wants to merge 5 commits into from

Conversation

SoucheSouche
Copy link
Collaborator

@SoucheSouche SoucheSouche commented Jul 4, 2025 •
edited
Loading

Checklist

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

Change description

Add linenoise to the list of extra components. The library is used in the console component of esp-idf that is going through refactoring. One part of the refactoring is to move third party libraries outside of the console component. Eventually, the console component itself will be removed from esp-idf, reason why this PR adds the linenoise lib to the idf-extra-components.

@SoucheSouche SoucheSouche changed the title Feat/add linenoise library Feat(linenoise): Add linenoise library Jul 4, 2025
@SoucheSouche SoucheSouche changed the title Feat(linenoise): Add linenoise library feat(linenoise): Add linenoise library Jul 4, 2025
@suda-morris suda-morris requested a review from o-marshmallow July 4, 2025 07:34
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 4, 2025
View reviewed changes
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

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

clang-tidy found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch from 859aa9c to af22955 Compare July 4, 2025 07:39
@github-actions github-actions bot changed the title feat(linenoise): Add linenoise library feat(linenoise): Add linenoise library (IEC-328) Jul 4, 2025
@suda-morris
Copy link
Collaborator

There was a PR about the linenoise library: espressif/esp-idf#10526
WDYT? @SoucheSouche

@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch 4 times, most recently from 019e413 to b4d3a0c Compare July 8, 2025 12:07
@SoucheSouche
Copy link
Collaborator Author

SoucheSouche commented Jul 8, 2025 •
edited
Loading

There was a PR about the linenoise library: espressif/esp-idf#10526 WDYT? @SoucheSouche

@suda-morris
Yes, I am aware of it. I took into account Ivan's comment left in the PR when implementing this version of linenoise. The author of the PR you mentioned is aware that I will take over and continue the work on improving the console component (linenoise and more generally the list of idea he listed here espressif/esp-idf#9986)

igrr
igrr requested changes Jul 8, 2025
View reviewed changes
@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch 7 times, most recently from e2dc2cc to 0a0b3e9 Compare July 14, 2025 13:59
@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch 4 times, most recently from 68556ce to 4530c52 Compare July 16, 2025 08:37
@SoucheSouche
Copy link
Collaborator Author

@igrr, I updated the component based on your input. I left a few comments open for discussion.

I added a bunch of tests for the basic functionality of linenoise when escape sequences are supported by the terminal.

@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch 3 times, most recently from f564777 to bca4fd1 Compare July 22, 2025 07:19
@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch 5 times, most recently from 7e0d7ad to e310ea1 Compare September 1, 2025 07:25
@SoucheSouche
Copy link
Collaborator Author

@igrr, could you please take another look, and check the comments I left opened?

@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch 2 times, most recently from 9a3c98d to 1021a72 Compare September 26, 2025 09:06
@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch from 1021a72 to 90bffac Compare October 2, 2025 06:51
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 2, 2025
View reviewed changes
@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch 3 times, most recently from cd20c01 to 61895b4 Compare October 3, 2025 06:53
@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch 8 times, most recently from 1dcb5b9 to ff761ab Compare October 10, 2025 12:22
@SoucheSouche SoucheSouche force-pushed the feat/add-linenoise-library branch from ff761ab to 2107edf Compare October 10, 2025 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@suda-morris suda-morris suda-morris left review comments

@igrr igrr igrr approved these changes

@o-marshmallow o-marshmallow Awaiting requested review from o-marshmallow

+1 more reviewer

@ESP-Marius ESP-Marius ESP-Marius left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@SoucheSouche SoucheSouche

Labels

Component: linenoise Status: Reviewing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@SoucheSouche @suda-morris @CLAassistant @igrr @ESP-Marius @espressif-bot @anastasia-be