Skip to content

Conversation

@miles-to-go
Copy link
Contributor

@miles-to-go miles-to-go commented Oct 14, 2025

Thanks again for this great readline library. One issue I noticed is that there was some slowness, which is especially noticeable when pasting text on slower terminals such as the GNOME Terminal.

I spent a little time trying to improve this; below are descriptions of each commit. If this PR is accepted and you are open to it, I will try to implement additional improvements in the future.

  • minimize allocs in line.go: Change core line buffer operations to not allocate in the default case (only if more space is needed)
  • preallocate capacity for slices: Pre-allocate the slices to a known capacity in matchBind
  • add DisplayLine test cases: Write a unit test for DisplayLine
  • combine DisplayLine prints: Update DisplayLine to make a single fmt.Print call
  • bracketed paste: Support bracketed paste mode so that pastes do not cause a display refresh - this is the main performance improvement for pastes just because it skips the refresh, but the other commits should make the library feel a little more responsive as well

@miles-to-go miles-to-go force-pushed the performance-improvements branch from 76423af to bee9f1e Compare October 14, 2025 21:03
@maxlandon
Copy link
Member

maxlandon commented Oct 23, 2025

Hello !

Thanks a lot for this PR, it is really appreciated!

At first glance, most commits will be merged.

I just need to check the one combining all DisplayLine calls in a single fmt.Print, because I'm pretty sure there is a reason why I did not do it in the first place.
Did you notice any chance occurring because of these modifications ?

I'm quite busy at work right now but I will soon merge this. Feel free to open new PRs with other improvements, these are always welcome !

@codecov
Copy link

codecov bot commented Oct 23, 2025

Codecov Report

❌ Patch coverage is 83.33333% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 33.03%. Comparing base (d3ac1d6) to head (0637db4).
⚠️ Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
emacs.go 0.00% 11 Missing ⚠️
readline.go 0.00% 3 Missing ⚠️
internal/keymap/dispatch.go 0.00% 1 Missing ⚠️
internal/strutil/key.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #87      +/-   ##
==========================================
- Coverage   33.04%   33.03%   -0.01%     
==========================================
  Files          57       57              
  Lines        9110     9109       -1     
==========================================
- Hits         3010     3009       -1     
  Misses       6062     6062              
  Partials       38       38              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@miles-to-go
Copy link
Contributor Author

I have not noticed any changes so far with the fmt.Printf change in DisplayLine, but I can try testing specific cases if needed. Also happy to remove that commit for now if it makes it easier to review. Thanks for the quick response!

@maxlandon
Copy link
Member

No need to remove this commits.
I will try writing a few tests around the prompt engine: it's been a while it should have some.

Will tackle this in a week. Try your luck if you want !

@maxlandon maxlandon enabled auto-merge January 14, 2026 04:47
@maxlandon maxlandon merged commit f819680 into reeflective:master Jan 14, 2026
9 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.

2 participants