[Terminal] Adding a terminal to Cadmus by KacperLa · Pull Request #77 · OGKevin/cadmus

KacperLa · 2026-01-25T00:48:36Z

THIS IS A WORK IN PROGRESS BUT IM PUTTING IT HERE INCASE PEOPLE WANNA TRY IT

I was working off of the head of Plato so there are a few extra commits from the HEAD of Plato, not sure if you are planning on syncing upstream changes

This pull request introduces a new terminal emulator feature, including all the necessary backend, UI, and keyboard integration. It also adds support for more keyboard keys and events. The most significant changes are the addition of the terminal view and its supporting modules, enhancements to keyboard handling (including new key kinds and events), and the integration of external crates to support terminal emulation and PTY functionality.

Terminal Emulator Integration

Added a new terminal module with submodules for buffer management, VT100 emulation, PTY handling, rendering, and session management. The main entry point is Terminal in session.rs. (crates/core/src/view/terminal/buffer.rs [1] crates/core/src/view/terminal/emulator.rs [2] crates/core/src/view/terminal/pty.rs [3] crates/core/src/view/terminal/mod.rs [4])
Registered the terminal as a new app command (AppCmd::Terminal) and view (ViewId::Terminal), and added it to the main menu. (crates/core/src/view/mod.rs [1] [2] crates/core/src/view/common.rs [3])

Keyboard and Input Handling Enhancements

Introduced new KeyKind variants for Tab, Escape, Control, and Arrow keys, including deserialization and display logic. (crates/core/src/view/key.rs [1] [2] [3] [4])
Updated keyboard state and event handling to support the new keys, including generating corresponding raw byte sequences for terminal input and handling Control key combinations. (crates/core/src/view/keyboard.rs [1] [2] [3] [4] [5])
Extended KeyboardEvent with Raw and Control variants to allow sending raw byte sequences and control characters. (crates/core/src/view/mod.rs)
Updated input field and key view logic to handle new keyboard events. (crates/core/src/view/input_field.rs [1] crates/core/src/view/key.rs [2])

Dependency and Version Updates

Added vt100 (via a patched branch for extra features) and portable-pty as dependencies to support terminal emulation and PTY spawning. (Cargo.toml [1] crates/core/Cargo.toml [2])

OGKevin · 2026-01-25T06:00:56Z

I was working off of the head of Plato so there are a few extra commits from the HEAD of Plato, not sure if you are planning on syncing upstream changes

Upstream was sycned last in 58b64f9.
You can drop the commits before 0b50d03.

KacperLa · 2026-01-27T01:24:21Z

@OGKevin I cleaned up the diff, let me know what you think.

OGKevin

Thanks for the PR and kudos!

I did an initial pass, I have some feedback:

I would appriciate if you can add some rust docs, especially a high level one on the module, and for most/all of the public API. Also sone of the internals that you deem worthy. Feel free to use an LLM for it, just make sure its not too sloppy and accurate.
I am of the opinion, that if you need inline comment, that logic should be extracted into a dedicated documented function, could you apply this principle please. (I'll make sure to create contributing instructions as well)

On the keyboard, when you press shift, there is a clear indicator that your next press is going to be a of the shift layer. With the introduction of the control key, this is not the same. When you press control, there is no indicator that the next key will be with pressed with the control modifier.
Would it make sense to add an indicator?

If you could rebase on top of 2935916, I can enable the CI. I believe cargo clippy might have some feedback :D

OGKevin · 2026-01-29T18:31:26Z

+            Event::WakeUp => {
+                if let Ok(mut buffer) = self.double_buffer.lock() {
+                    if buffer.is_dirty() {
+                        if buffer.take_full_refresh() {
+                            rq.add(RenderData::no_wait(self.id, self.rect, UpdateMode::Gui));
+                        } else {
+                            for dirty_rect in buffer.drain_dirty_rects() {
+                                let update_rect = Rectangle::new(
+                                    Point::new(self.rect.min.x + dirty_rect.min.x, self.rect.min.y + dirty_rect.min.y),
+                                    Point::new(self.rect.min.x + dirty_rect.max.x, self.rect.min.y + dirty_rect.max.y),
+                                );
+                                rq.add(RenderData::no_wait(self.id, update_rect, UpdateMode::FastMono));
+                            }
+                        }
+                    }
+                }
+                true
+            }


What is the reason for capturing this event and not let it continue up the chain?

OGKevin · 2026-01-29T18:38:22Z

+        let saved_layout = context.settings.keyboard_layout.clone();
+        context.settings.keyboard_layout = "Terminal".to_string();
+
+        let mut kb_rect = rect![
+            rect.min.x,
+            rect.max.y - (small_height + 3 * big_height) as i32 + big_thickness,
+            rect.max.x,
+            rect.max.y
+        ];
+        let keyboard = Keyboard::new(&mut kb_rect, false, context);
+        children.push(Box::new(keyboard) as Box<dyn View>);
+
+        context.settings.keyboard_layout = saved_layout;
+


This feels a bit dirty haha. But I guess this is needed due to no way of telling the keyboard which layout to render directly.

I suggest extracting this into a function with #[inline] so it can be documented.

OGKevin · 2026-01-29T18:39:22Z

+        let (_, big_thickness) = halves(thickness);
+
+        let saved_layout = context.settings.keyboard_layout.clone();
+        context.settings.keyboard_layout = "Terminal".to_string();


Could you introduce an enum for this please 🙏

OGKevin · 2026-01-29T18:41:23Z

+        let emulator_shared = Arc::new(Mutex::new(Emulator::new(rows, cols)));
+        let shutdown_flag = Arc::new(AtomicBool::new(false));
+
+        let hub = hub.clone();
+        let buffer_shared = Arc::clone(&double_buffer);
+        let emulator_shared = Arc::clone(&emulator_shared);


Am I missing something 👀

Whats the reason for 2x emulator_shared?

OGKevin · 2026-01-29T18:45:17Z

+                                Err(e) => {
+                                    eprintln!("Failed to create terminal: {}", e);
+                                    continue;
+                                }


Since this is the emulator, lets just panic 💣

OGKevin · 2026-01-29T18:52:13Z

                EntryId::Launch(AppCmd::Calculator),
            ),
            EntryKind::Command("Sketch".to_string(), EntryId::Launch(AppCmd::Sketch)),
+            EntryKind::Command("Terminal".to_string(), EntryId::Launch(AppCmd::Terminal)),


Can you introduce an enum here as well.
The terminal command can be the first and only entry. And you can use to_string() on it.

Add a todo comment so that later on it can be refactored.
So that EntryKind::Command takes the enum instead of strings.

I have a feeling this suggestion should be a separate issue. Maybe I am misunderstanding the ask but my instinct is that I want to keep this diff as specific to the terminal application as possible.

OGKevin · 2026-01-29T18:54:03Z

+        let dpi = CURRENT_DEVICE.dpi;
+        let font_size_scaled = (font_size * 64.0) as u32;
+
+        // Setup menu icon in top right corner


I would appriciate if you extract this into a dedicated function using #[inline] and document that function if needed.

OGKevin · 2026-01-29T18:54:40Z

+        .corners(Some(CornerSpec::Uniform(border_radius)));
+        children.push(Box::new(icon) as Box<dyn View>);
+
+        // Add terminal keyboard


Same ask here and all the ones that follow.

I believe it becomes easier to follow to break this big function down into smaller ones with #[inline].

OGKevin · 2026-01-29T18:56:13Z

+
+const ICON_NAME: &str = "enclosed_menu";
+
+pub struct Terminal {


Can you add some rust docs explaning the public API.
I am also fine if you decide to use an LLM for it, just make sure its not too sloppy.

As I read the code I need to go throught the thinking process that you might have already done.
So it would be nicer to document it, instead of having everyone do the same thinking process!

OGKevin · 2026-01-29T19:01:47Z

@@ -0,0 +1,7 @@
+mod buffer;


Can you add a high level rustdoc for the entire module 🙏

feat: add terminal font size setting and update terminal initialization fix: correct key mapping for Tab in terminal layout fix: dirty rectangle management in DoubleBuffer and session, this should eliminate the issue with characters being left behind on the screen when many frames are pushed in a short period of time.

OGKevin marked this pull request as draft January 25, 2026 05:53

KacperLa force-pushed the kl_terminal branch 2 times, most recently from a73a4da to d84fb3c Compare January 27, 2026 01:04

KacperLa changed the title ~~[WIP][Terminal] Adding a terminal to Cadmus~~ [Terminal] Adding a terminal to Cadmus Jan 27, 2026

OGKevin marked this pull request as ready for review January 27, 2026 04:03

OGKevin self-requested a review January 27, 2026 04:03

OGKevin requested changes Jan 29, 2026

View reviewed changes

OGKevin added this to Cadmus Mar 29, 2026

github-project-automation Bot moved this to Backlog in Cadmus Mar 29, 2026

KacperLa force-pushed the kl_terminal branch from fd9f988 to e7d4174 Compare July 11, 2026 23:55

dosubot Bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jul 11, 2026

KacperLa force-pushed the kl_terminal branch from e7d4174 to 00c2345 Compare July 12, 2026 01:47

OGKevin mentioned this pull request Jul 12, 2026

ci: no need to pass ref in crowdin checkout #670

Merged

KacperLa force-pushed the kl_terminal branch from 00c2345 to 411543f Compare July 12, 2026 23:20

KacperLa added 8 commits July 16, 2026 12:02

PR review comments fixes

df093f2

feat(terminal): improve interaction and resizing

1b4c129

refactor(terminal): address PR review feedback

3c3a7bc

fix(terminal): address rendering and lifecycle review

50d1244

fix(terminal): preserve output and render state

c786425

fix(terminal): reconstruct buffer after resize

bf0a638

test(terminal): exercise session lifecycle and rendering

6d19807

KacperLa force-pushed the kl_terminal branch from 411543f to 6d19807 Compare July 16, 2026 18:49

dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jul 16, 2026

This comment was marked as spam.

Sign in to view

refactor(terminal): own keyboard sequence encoding

1f4b795


		const ICON_NAME: &str = "enclosed_menu";

		pub struct Terminal {

Conversation

KacperLa commented Jan 25, 2026

THIS IS A WORK IN PROGRESS BUT IM PUTTING IT HERE INCASE PEOPLE WANNA TRY IT

Uh oh!

OGKevin commented Jan 25, 2026

Uh oh!

KacperLa commented Jan 27, 2026

Uh oh!

OGKevin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as spam.

This comment was marked as spam.

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants