Skip to content
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.

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

feat: Find in Editor #295

Merged
merged 41 commits into from
Apr 8, 2025
Merged

feat: Find in Editor #295

merged 41 commits into from
Apr 8, 2025

Conversation

tom-ludwig
Copy link
Member

@tom-ludwig tom-ludwig commented Mar 13, 2025

Important

We need to merge CodeEditApp/CodeEditTextView#78 before merging this PR.

Description

This PR introduces the initial implementation of the “Find in Editor” feature for the source editor. Users can now search for text within the currently open file using ⌘ F. All matching results are visually emphasized, and users can navigate between matches using next/previous controls.

What’s Included

  • Text search across the current document
  • Match highlighting with emphasis on the currently selected match
  • Keyboard shortcut support: ⌘ F to activate the find bar
  • Looping navigation with HUD notifications:
    • Reaching the end → loops to first result (arrow.triangle.capsulepath)
    • Reaching the beginning → loops to last result (flipped arrow.triangle.capsulepath)
    • No more matches → arrow.down.to.line HUD icon displayed

Related Issues

  • #ISSUE_NUMBER

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

@austincondiff austincondiff changed the title feat: Search in Document feat: Find in Editor Mar 16, 2025
thecoolwinter and others added 11 commits March 16, 2025 11:11
…hen looping find results or submitting when there is no result. Commented emphasize logic back in.
- If the users cursor is in view, the highlighted item will be next to
  the cursor
- If the users cursor is out of view, we'll highlight the closed item to
  the visibe area
…match cycle logic from EmphasisManager to FindViewController. Using EmphasisManager in bracket pair matching instead of custom implementation reducing duplicated code. Implemented flash find matches when clicking the next and previous buttons when the editor is in focus. `bracketPairHighlight` becomes `bracketPairEmphasis`. Fixed various find issues and cleaned up implementation.
…dings didnt work while find panel was in focus preventing it from being dismissed.
@thecoolwinter
Copy link
Collaborator

thecoolwinter commented Apr 7, 2025

Found one more bug that I'm working on in CodeEdit, I'm finding that the find panel is being removed from the view hierarchy (but not stopping being drawn) sometimes when clicking out of the find panel, and only when a previous tab had the find panel opened. I'm investigating and will report back.

As well as the failing tests.

@thecoolwinter
Copy link
Collaborator

Fixed the aforementioned bug, ended up strongly referencing self in the monitor closure. Not too sure why it was losing the reference, as the view it turned out was perfectly fine in memory. I added a call to removeEventMonitor in the view's deinit to ensure we don't leak memory.

@thecoolwinter
Copy link
Collaborator

As I'm fixing the tests, I've realized that the emphasis manager is not correctly drawing the non-flash emphases. I'm going to go fix that and link the PR here.

@thecoolwinter
Copy link
Collaborator

Okay I have a few updates:

  • I've fixed the drawing issues by adding a specific code path for drawing underlines.
  • I also found a bug where the emphasis manager was leaking emphases due to an offset bug.
  • During that, I inadvertently fixed the drawing code for drawing emphases that span multiple line fragments, which causes the bug shown in the video below.
  • However, during that change I was able to drastically simplify the 'rounded rect' code.
Screen.Recording.2025-04-07.at.12.11.10.PM.mov

I'm going to open the PR for these changes, and I'm tempted to just merge the change so it functions correctly and come back around to fix the text layer bug.

thecoolwinter added a commit to CodeEditApp/CodeEditTextView that referenced this pull request Apr 7, 2025
### Description

- Fixes an emphasis manager bug where it could leak layers without removing their `CALayer`s when replacing flash emphases with other emphases.
- Updates the emphasis drawing code to correctly draw underlines.
- Updates the `roundedPathForRange` to correctly handle emphases that span multiple line fragments.

While fixing the last point, I discovered that the emphasis manager doesn't correctly draw the text on emphases that span multiple line fragments. This makes sense, as right now it's just using a CATextLayer that has no knowledge of line breaks.

In the future, that text layer should be replaced by a custom layer that draws the text using knowledge of existing line fragments positions. However, I think the issues this PR fixes are too important to wait until that can be done.

Also note that that change will be critical to drag and drop, so it will absolutely be done and I have a branch somewhere with some of that work already completed.

### Related Issues

* CodeEditApp/CodeEditSourceEditor#295

### Checklist

- [x] I read and understood the [contributing guide](https://github.com/CodeEditApp/CodeEdit/blob/main/CONTRIBUTING.md) as well as the [code of conduct](https://github.com/CodeEditApp/CodeEdit/blob/main/CODE_OF_CONDUCT.md)
- [x] The issues this PR addresses are related to each other
- [x] My changes generate no new warnings
- [x] My code builds and runs on my machine
- [x] My changes are all related to the related issue above
- [x] I documented my code

### Screenshots

Screen recording showing the text layer bug mentioned above, as well as the fixed multi-fragment drawing, and the fixed leak.

Prior to this, when switching from flash to any other emphasis type, there was a good chance one of the new emphases would be kept. This was due to the emphasis manager assuming that the emphasis being flashed was kept around after the animation delay. This has been fixed in this PR.

https://github.com/user-attachments/assets/a29048e4-ab81-4376-abe0-e0499e448ea8
@thecoolwinter
Copy link
Collaborator

Alright this should be ready to go, I fixed a myriad of issues that only appeared when using multiple tabs, or the odd way we handle safe area in CodeEdit.

Specifically this patch now:

  • Correctly updates the scroll view offset every time it's updated.
  • Uses constants rather than string literals for emphasis group IDs.
  • Correctly draws underline and border bracket pair emphases.
  • Adds a new additionalInsets parameter that allows the text content to be inset by an additional amount than any decoration. Previously this was done on the CodeEdit side, but the lack of distinction led to the find panel having a transparent 2px above it. Moving it down here makes sense anyways.
  • Removed final references to 'highlight layers' from previous bracket emphasis method.
  • Added tests for the new parameter, and updated all broken tests.

@austincondiff
Copy link
Collaborator

Is the EmphasisGroup enum extendable? I want to think about the future when extensions use this API. That is why I initially used strings but an enum could be better here.

@thecoolwinter
Copy link
Collaborator

Is the EmphasisGroup enum extendable? I want to think about the future when extensions use this API. That is why I initially used strings but an enum could be better here.

It's not using enum cases, it's using static variables but enums can't be initialized, so there can't be any confusion by trying to 'create' one. So it's still using strings to identify groups.

I don't think an enum is the right decision here for the reason you mentioned.

@thecoolwinter
Copy link
Collaborator

It is extendable, but it's not public since I don't think there's really a point to exposing that API. Anyone using the package who wants to use the emphasis API will work directly with the EmphasisManager, not any methods in CESE. I created it so there's consistent use everywhere groups share IDs (like in tests).

@thecoolwinter thecoolwinter merged commit f444927 into main Apr 8, 2025
2 checks passed
@thecoolwinter thecoolwinter deleted the feat/in-doc-search branch April 8, 2025 17:19
thecoolwinter added a commit to CodeEditApp/CodeEdit that referenced this pull request Apr 8, 2025
…xes (#2020)

### Description

Updates CodeEditSourceEditor.

Includes:
- Find in Editor feature.
- Improved undo grouping.
- Improved word detection when selecting words.
- Improved overscroll behavior.
- Added mouse drag selection modes.

### Related Issues

* CodeEditApp/CodeEditTextView#1
* CodeEditApp/CodeEditSourceEditor#295

### Checklist

- [x] I read and understood the [contributing guide](https://github.com/CodeEditApp/CodeEdit/blob/main/CONTRIBUTING.md) as well as the [code of conduct](https://github.com/CodeEditApp/CodeEdit/blob/main/CODE_OF_CONDUCT.md)
- [x] The issues this PR addresses are related to each other
- [x] My changes generate no new warnings
- [x] My code builds and runs on my machine
- [x] My changes are all related to the related issue above
- [x] I documented my code
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.

✨ Improve Design Implementation for Find in File
3 participants