Skip to content

Conversation

@Mostamhd
Copy link
Contributor

@Mostamhd Mostamhd commented Feb 7, 2026

closes #1615

@Mostamhd Mostamhd requested a review from a team as a code owner February 7, 2026 02:21
@dgageot
Copy link
Member

dgageot commented Feb 7, 2026

Thanks for the PR @Mostamhd!
I left a few comments for potential improvements.

@dgageot
Copy link
Member

dgageot commented Feb 7, 2026

/review

github-actions[bot]
github-actions bot previously approved these changes Feb 7, 2026
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Review Summary

✅ No bugs found in the added code. The reverse history search implementation looks solid.

Analysis

Reviewed the new FindPrevContains and SetCurrent functions along with the editor integration. The code correctly:

  • Handles edge cases (empty history, negative indices, out of bounds)
  • Uses proper guards before calling SetCurrent (checks matchIdx >= 0)
  • Has comprehensive test coverage including boundary conditions
  • Matches the documented API contract for exclusive upper bound behavior

The implementation is well-tested and handles edge cases appropriately.

Signed-off-by: Moustafa Ahmed <mostamhd@proton.me>
@Mostamhd Mostamhd force-pushed the feature/ctrl-r-history-search branch from 3b1e115 to 931d90b Compare February 8, 2026 16:10
@Mostamhd
Copy link
Contributor Author

Mostamhd commented Feb 8, 2026

Hey @dgageot , thank you for the review!

I have refactored the code and worked on the comments you mentioned. Feel free to let me know if you have any questions or comments.

@dgageot dgageot merged commit 4ed21c2 into docker:main Feb 9, 2026
5 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.

Ctrl+R to search through the history

2 participants