Fix escape sequence problem

[yhirose]

This is to fix the problem reported in the following issue and pull request.

antirez/linenoise#129
antirez/linenoise#230

You can reproduce this problem with the following steps:

• Run the example program (mkdir build && cd build && meson .. && ninja && ./linenoise)
• Type 'h'
• Press TAB (it shows 'hello'.)
• Press CTRL-LEFT (it corrupts the buffer...)

This PR is based on antirez/linenoise#230. It almost works, but it actually creates another problem described in the first comment in antirez/linenoise#129. I applied the suggestion in the comment to check the readability of the input fd.

Summary by Sourcery

Fixes an issue where escape sequences could corrupt the input buffer when using tab completion and pressing CTRL-LEFT. This is achieved by reading escape sequences using a poll to check the readability of the input file descriptor.

Bug Fixes:

• Fixes a bug where escape sequences could corrupt the input buffer when using tab completion.
• Fixes an issue with handling escape sequences, especially when using CTRL-LEFT after tab completion, which was corrupting the buffer.

[sourcery-ai]

Reviewer's Guide by Sourcery

This pull request fixes an issue where pressing CTRL-LEFT corrupted the buffer during tab completion. It introduces a new function, readEscapeSequence, to properly read and interpret escape sequences from the input. The completeLine and linenoiseEditFeed functions are updated to use this new function and handle different escape sequences accordingly.

Sequence diagram for handling escape sequences in linenoiseEditFeed

sequenceDiagram
  participant linenoiseEditFeed
  participant readCode
  participant readEscapeSequence
  participant completeLine
  participant linenoiseEditMoveLeft

  linenoiseEditFeed->>readCode: Reads input code
  alt c == ESC
    linenoiseEditFeed->>readEscapeSequence: Reads escape sequence
    readEscapeSequence-->>linenoiseEditFeed: Returns esc_type
  end
  alt esc_type == ESC_LEFT
    linenoiseEditFeed->>linenoiseEditMoveLeft: Moves cursor left
  else esc_type == ESC_RIGHT
    linenoiseEditFeed->>linenoiseEditMoveRight: Moves cursor right
  else esc_type == ESC_HOME
    linenoiseEditFeed->>linenoiseEditMoveHome: Moves cursor to home
  else esc_type == ESC_END
    linenoiseEditFeed->>linenoiseEditMoveEnd: Moves cursor to end
  else esc_type == ESC_UP
    linenoiseEditFeed->>linenoiseEditHistoryNext: Retrieves previous history entry
  else esc_type == ESC_DOWN
    linenoiseEditFeed->>linenoiseEditHistoryNext: Retrieves next history entry
  else esc_type == ESC_DELETE
    linenoiseEditFeed->>linenoiseEditDelete: Deletes character at cursor
  else c == TAB and completionCallback != NULL
    linenoiseEditFeed->>completeLine: Handles tab completion
    completeLine-->>linenoiseEditFeed: Returns character
  end

Loading

Sequence diagram for handling tab completion in completeLine

sequenceDiagram
  participant completeLine
  participant linenoiseCompletions
  participant linenoiseBeep
  participant refreshLine

  completeLine->>linenoiseCompletions: Generates completions
  alt TAB key pressed
    completeLine->>completeLine: Updates completion index
    alt completion_idx == lc.len
      completeLine->>linenoiseBeep: Beeps to indicate end of completions
    end
  else ESC key pressed
    completeLine->>refreshLine: Re-shows original buffer
  else other key pressed
    completeLine->>completeLine: Updates buffer with completion
  end
  completeLine->>refreshLine: Shows completion or original buffer

Loading

File-Level Changes

Change	Details	Files
Introduces a new function to read escape sequences from the input file descriptor.	Adds a poll call to check for readability of the input file descriptor before reading escape sequence bytes. Defines an enum ESC_TYPE to represent different escape sequences. Implements readEscapeSequence to read and interpret escape sequences, handling timeouts and errors. Handles 'ESC [', and 'ESC O' sequences.	linenoise.cpp
Modifies the completeLine function to handle escape sequences during tab completion.	The completeLine function now accepts an ESC_TYPE argument. The completeLine function now checks for the ESC key and ESC_TYPE to determine if the original buffer should be re-shown. Removes the switch statement that handles TAB and ESC keys.	linenoise.cpp
Updates linenoiseEditFeed to use the new readEscapeSequence function and handle different escape sequences.	Calls readEscapeSequence when an ESC character is read. Uses a switch statement to handle different ESC_TYPE values, calling appropriate functions like linenoiseEditDelete, linenoiseEditMoveHome, etc. Removes the logic for reading and interpreting escape sequences directly within linenoiseEditFeed.	linenoise.cpp

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!
• Generate a plan of action for an issue: Comment @sourcery-ai plan on
  an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey @yhirose - I've reviewed your changes - here's some feedback:

Overall Comments:

• Consider using a non-blocking read instead of poll with a timeout in readEscapeSequence.
• The readEscapeSequence function could be improved by adding a loop to read all available bytes, ensuring that the entire escape sequence is processed.

Here's what I looked at during the review

• 🟢 General issues: all looks good
• 🟢 Security: all looks good
• 🟢 Testing: all looks good
• 🟡 Complexity: 1 issue found
• 🟢 Documentation: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (complexity): Consider extracting the escape handling logic into its own function to reduce nesting and improve code readability in both the completion and feed paths, isolating ESC_TYPE handling and simplifying the primary control flow in linenoiseEditFeed and completeLine functions..

Consider extracting the escape handling into its own function to flatten the nesting in both the completion and feed paths without reverting the abstractions. This isolates the ESC_TYPE handling and makes the primary control flow easier to follow. For example, you might refactor the ESC switch logic in linenoiseEditFeed as follows:

static void handleEscape(linenoiseState *l, ESC_TYPE esc_type) {
    switch (esc_type) {
    case ESC_DELETE:
        linenoiseEditDelete(l);
        break;
    case ESC_UP:
        linenoiseEditHistoryNext(l, LINENOISE_HISTORY_PREV);
        break;
    case ESC_DOWN:
        linenoiseEditHistoryNext(l, LINENOISE_HISTORY_NEXT);
        break;
    case ESC_RIGHT:
        linenoiseEditMoveRight(l);
        break;
    case ESC_LEFT:
        linenoiseEditMoveLeft(l);
        break;
    case ESC_HOME:
        linenoiseEditMoveHome(l);
        break;
    case ESC_END:
        linenoiseEditMoveEnd(l);
        break;
    default:
        break;
    }
}

Then, in linenoiseEditFeed reduce nesting by calling:

if (c == ESC) { 
    auto esc_type = readEscapeSequence(l);
    handleEscape(l, esc_type);
    break;
}

Similarly, in your completeLine function, consider isolating the escape-specific branch into a helper if it gets too nested. This approach keeps all functionality intact while simplifying the main control flow.

[ericcurtin]

I think this is a good idea to suggest smaller functions like handleEscape . Would like to see this in a follow on PR.

[sourcery-ai]

I have created an issue for your comment: #13