Skip to content

Review Console reference management#1161

Open
lionel- wants to merge 26 commits intotask/sync-plot-commfrom
task/sync-plot-comm-2
Open

Review Console reference management#1161
lionel- wants to merge 26 commits intotask/sync-plot-commfrom
task/sync-plot-comm-2

Conversation

@lionel-
Copy link
Copy Markdown
Contributor

@lionel- lionel- commented Apr 22, 2026

Branched from #1100
Progress towards #1145

  • New Console::with() that passes a Console ref and catches panics, transforming them to Result.

  • Console::get() pushed to the FFI boundary and abstracted with [ark::register]. This new macro wraps [harp::register] and calls Console::with(). Any panics due to e.g. RefCell issues gets converted to an R error.

  • The graphics callbacks are migrated to [ark::register], but all Ark FFI entries should use it in the future.

  • DeviceContext no longer reaches back into Console. It stores all needed state on itself.

  • PlotComm also owns what it needs. I was wrong to pass &Console to all handlers. PlotComm is the only CommHandler that used &Console, just to get the device_context(). It now holds an Rc<DeviceContext> passed at construction time.

  • All methods in console_comm.rs now take &self. Supported by a take/remove pattern. Also the comms are now stored in a DebugRefCell, a new type that maps to RefCell in debug builds and to UnsafeCell (with logging) in release builds. This allows us to transition to more Rust safety without causing panics in release builds, and without having to handle panics in awkward places.

@lionel- lionel- force-pushed the task/sync-plot-comm-2 branch from 6774491 to 45464f3 Compare April 22, 2026 10:08
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.

1 participant