Rename interface.rs to console.rs and RMain to Console
#1024
Conversation
lionel-
force-pushed
the
task/rename-interface
branch
from
0819c21 to
72fa9d0
Compare
DavisVaughan
left a comment
DavisVaughan
left a comment
There was a problem hiding this comment.
I think we should put in the effort to complete the rename. i.e. let main should become let console everywhere. And I'd prefer if con could be console, for clarity
| } | ||
|
|
||
| let main = RMain::get(); | ||
| let main = Console::get(); |
There was a problem hiding this comment.
You're going to have a lot of these, which are going to be quite confusing to read 😢
If you can get these to update to let console = then I'd be in favor of that
| #[cfg(not(test))] // Unit tests do not have an `RMain` | ||
| // Mem-Safety: Object protected by `RMain` for the duration of the `r_task()` | ||
| let mut env = crate::interface::RMain::get().read_console_frame().sexp; | ||
| #[cfg(not(test))] // Unit tests do not have an `Console` |
There was a problem hiding this comment.
| #[cfg(not(test))] // Unit tests do not have an `Console` | |
| #[cfg(not(test))] // Unit tests do not have a `Console` |
crates/ark/src/lsp/backend.rs
Outdated
| let events_tx = events_tx.clone(); | ||
| move || { | ||
| RMain::with_mut(|main| main.set_lsp_channel(events_tx)); | ||
| Console::with_mut(|con| con.set_lsp_channel(events_tx)); |
There was a problem hiding this comment.
I greatly prefer console over con. I immediately think "connection" when I see con
There was a problem hiding this comment.
I see where you're coming from. That said the con abbreviation for console is a very common one: https://github.com/search?q=%22con+%3D+console%22&type=code
Another abbrev I've been using for connection is "conn".
For readability I think it's important to have an abbreviation for small scopes especially tiny ones like this lambda.
| // For shutdown signal in integration tests | ||
| pub static CLEANUP_SIGNAL: (Mutex<bool>, Condvar) = (Mutex::new(false), Condvar::new()); | ||
|
|
||
| pub fn setup_r(args: &Vec<String>) { |
There was a problem hiding this comment.
I'm going to trust that nothing changed in here beyond moving it from interface.rs to console.rs 😄
lionel-
force-pushed
the
task/rename-interface
branch
from
72fa9d0 to
27acaae
Compare
lionel-
force-pushed
the
task/rename-interface
branch
from
27acaae to
b655078
Compare
The core of Ark has historically been implemented in
interface.rsand consolidated in a singleton structRMainin #52Really this struct is mostly about the ReadConsole and WriteConsole handlers, and we've been thinking about renaming
interface.rstoconsole.rs, andRMaintoConsolefor a while. Some files already use theconsole_prefix, e.g.console_debug.rsandconsole_annotate.rs. This PR gets us through the other side of that ongoing change.