Open thscharler opened 1 week ago
Attention: Patch coverage is 72.72727%
with 3 lines
in your changes missing coverage. Please review.
Project coverage is 94.4%. Comparing base (
7c0665c
) to head (51c0a05
). Report is 1 commits behind head on main.
Files | Patch % | Lines |
---|---|---|
src/buffer/buffer.rs | 62.5% | 3 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
As a brief initial reaction, I'm not certain I like this idea, because it feels like we're just pushing more random stuff into the buffer without a strong plan for how this looks and thinking about where things belong. I need to give this some proper thought, but I wanted to make sure that this was noted for the other devs considering this PR.
Fine with me.
One more thought why it might be fitting: I create a secondary buffer, eg for a viewport and hand it down somewhere to render. Then I relocate the result and copy it in the main buffer. In this case it would be necessary to remap the cursor position too.
In the end there is a link between the buffer and the cursor, as both directly contribute to what the terminal displays.
But I agree that this is not a general solution for other context values.
Maybe one way to go with #1044 is to recognize that there is already a context struct passed into render, and it's named Buffer.
I gave this a try and added cursor_pos, and it doesn't fit too badly.
--
The current state is quite a chore, when writing an application with more then 3 text-input fields. You have to collect all the possible cursor-positions, check against the current focus and set the cursor. and all this in the application code.