Any chance we can address the #47093 (review) comment?

I checked that old review comment again. This PR does add the missing failing regression coverage in file_finder for the Unicode path:line:column case.

The red CI is/was a separate regression on the existing out-of-bounds behavior, which I’ve now fixed (I think) by switching these open path flows to anchor based nav so EOF still clips correctly.

I walked through the CI failures and pushed a follow-up fix.

Changes in the update:
• removed the unused text dependency that was tripping cargo-machete
• stopped using raw buffer points for these external path:line:column navigation paths
• switched those call sites to anchor-based navigation so EOF clipping still works correctly

That preserves the Unicode column fix from this PR and fixes the separate out-of-bounds regression that CI caught.

I'm still fairly new to the codebase, so if I missed anything or took the wrong approach, I’d appreciate the correction. Thanks!

No, that review comment asks to alter another module to reuse the same new code:

No, that review comment asks to alter another module to reuse the same new code:

You were right. I misread that old review comment three times. Honestly, I think I just looked right over it to the last item in the thread. My mistake.

I now realize that go_to_line still had its own UTF-8 column conversion path. I’ve switched go_to_line to use the same external row/column conversion approach as the open path flows, added multibuffer coverage for the shared path, reran the targeted tests, and manually verified the behavior with a stateless build on my local machine.

Thanks for the correction and my bad for the oversight.

Thanks @SomeoneToIgnore. These were all good calls and exactly the kind of thing I need this early into the codebase. I will eliminate the waste as I get more familiar - thank you. So, I tightened the impl and pushed the follow up...

• restored the external nav call sites to go_to_singleton_buffer_point
• fixed the underlying EOF clipping issue in the point based path instead of keeping the temporary anchor based editor API
• removed the extra MultiBufferSnapshot external-column helper and kept the shared conversion at the text layer
• bounded the conversion scan using the UTF8 upper bound
• clipped the bounded end point through the buffer APIs so the helper never slices through a multibyte
• kept go_to_line UTF8 correct without broadening the API surface

I also reran the stateless zed /path/to/unicode.txt:1:1/5/10/999 launch path locally after the final fix and it's good.

If I've made errors or introduced waste/inefficiencies that you see - point them out and I'll circle back. Thanks a lot!

Note Obviously, the PR body is a bit stale. If you need it updated... let me know, but I imagine we're both on the same page - yes?

Edit Rebased onto current main and fixed a manifest conflict that surfaced

You're welcome!

I noticed that the 'Tidy Up' commit looks to have broken the way the cursor mapping works, though? I think we need to restore inclusive endpoint handling in buffer_point_to_anchor, yeah?

The expected clamping behavior is defined in the test such that if the row/column are out of file bounds, they should clamp to last valid position (last row/last column). Isn't that the desired behavior here?

Thanks for noticing the culprit of the test failure: I have not found that particular change clear hence tried to revert.
Excerpts have exclusive ranges, so using = there feels wrong, as wrong is the fact that the 2nd conversion method was not updated to replicate the behavior.

I'm trying to think of other way to fix this all now.
Seems that file finder's confirm passes 0:18 correctly into go_to_singleton_buffer_point during this test?
I wonder if some .min or .clip_point or fallback is missing somewhere in the new calculation code?
Would be superb to hear your ideas and get a fix 🙂

Tips Hat

Thanks for working with me, @SomeoneToIgnore. It's been a pleasure. After seeing the update, I understand. Thanks again!