Skip to content

Conversation

@SoucheSouche
Copy link
Collaborator

@SoucheSouche SoucheSouche commented Oct 28, 2025

Checklist

  • CI passing

Change description

  • Added getters for out_fd and in_fd config structure fields
  • Removed self field in esp_linenoise_instance_t and related sanity check performed on it
  • Added NULL pointer checks in public API
  • Fixed typo in macro

@SoucheSouche SoucheSouche force-pushed the fix/esp_linenoise_findings branch from d0d63ca to 1a599d6 Compare October 28, 2025 01:31
@suda-morris suda-morris requested a review from Copilot October 28, 2025 04:05
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 addresses minor bugs and code quality improvements in the esp_linenoise component, including fixing a macro naming typo, removing an unnecessary self-reference field, and adding getter functions for file descriptors.

  • Fixed macro naming from esp_LINENOISE_CHECK_INSTANCE to ESP_LINENOISE_CHECK_INSTANCE
  • Removed the self field from esp_linenoise_instance_t structure and its associated sanity check
  • Added getter functions esp_linenoise_get_out_fd() and esp_linenoise_get_in_fd() with test coverage

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
esp_linenoise/src/esp_linenoise.c Fixed macro naming, removed self-reference assignment, added inline getter functions for file descriptors
esp_linenoise/private_include/esp_linenoise_private.h Removed self field from instance structure and its initialization
esp_linenoise/include/esp_linenoise.h Added documentation for new getter functions
esp_linenoise/test_apps/main/test_esp_linenoise_get_set.c Added test case for new file descriptor getter functions
esp_linenoise/idf_component.yml Version bump from 1.0.0 to 1.0.1

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

@SoucheSouche SoucheSouche force-pushed the fix/esp_linenoise_findings branch from 1a599d6 to 98b5404 Compare October 28, 2025 05:56
@SoucheSouche SoucheSouche force-pushed the fix/esp_linenoise_findings branch from 98b5404 to 579ce6d Compare October 30, 2025 01:10
@SoucheSouche SoucheSouche force-pushed the fix/esp_linenoise_findings branch from 52f3cae to f58574a Compare November 6, 2025 11:17
@SoucheSouche
Copy link
Collaborator Author

@suda-morris, feel free to merge if the CI is green.

@SoucheSouche SoucheSouche force-pushed the fix/esp_linenoise_findings branch from f58574a to 9a556cf Compare November 6, 2025 11:55
@ESP-Marius ESP-Marius merged commit 696a3cf into espressif:master Nov 7, 2025
161 of 162 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants