Rendering - thorough overhaul of how sericom-core handles rendering.#43
Open
Rendering - thorough overhaul of how sericom-core handles rendering.#43
Conversation
changelog: ignore (sericom-core)
good checkpoint, stuff is pretty much solid regarding color parsing from escape sequences and building lines/spans from the parsed. Only major changes would come from needing to work with cursor movements. TODO - handle cursor movement. changelog: ignore
changelog: ignore
changelog: ignore
Bumps [clap](https://github.com/clap-rs/clap) from 4.5.50 to 4.5.53. - [Release notes](https://github.com/clap-rs/clap/releases) - [Changelog](https://github.com/clap-rs/clap/blob/master/CHANGELOG.md) - [Commits](clap-rs/clap@clap_complete-v4.5.50...clap_complete-v4.5.53) --- updated-dependencies: - dependency-name: clap dependency-version: 4.5.53 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Good checkpoint. Instead of one monster `ScreenBuffer::process_events` function I added `ScreenDriver` which takes the `ParserEvents` and `ScreenBuffer` to handle the processing in a cleaner fashion. Still need to tie up loose ends regarding the actual implementation of processing the incoming `ParserEvents` and then figure out how to actually print to the screen. changelog: ignore
tkatter
added
refactor
Contributor
🧪 CI InsightsHere's what we observed from your CI run for d0bebbe. 🟢 All jobs passed!But CI Insights is watching 👀 |
Worked out some of the knots in the cursor/color escape sequence handling and line/span manipulation. The current handling of parsed events passes the old unit tests which is good - pretty close to being up to speed. Last main todo before wiring it all up is handle screen erasure escape sequences. There are a few things that I have skipped (\r\n && screen modes) but I'm going to roll without it until I get further along and get a chance to test what is/isn't implemented based on real-world results. Did some more file re-structuring, cleaned up a bunch of lints, and cleaning out unused methods is a WOP. Updated the justfile and the test-sericom crate. changelog: ignore
tkatter
commented
Nov 24, 2025
tkatter
commented
Nov 24, 2025
tkatter
commented
Nov 24, 2025
tkatter
commented
Nov 24, 2025
Owner
Author
|
in |
I was writing code (specifically for handling the clear [screen|line] escape sequences when I realized that `crossterm` has some nice things I could use here instead of implementing it myself and facing (potentially) more bugs. I was about to delete nearly everything I had written for that function when I realized the code could probably be used when not writing to a terminal backend. I've been planning to make a GUI version with a different feature set and would not have a terminal for crossterm to render to. So I'm making this commit before I start screwing around with feature flags. changelog: ignore
Figured out that I could just implement crossterm's `Command` trait to make rendering easier for the cli. changelog: ignore
…ring I remember now why I had the `ScreenBuffer::set_char_at_cursor()` method - for maintaining spacing and allowing easy overwriting. Will need to bring this method, along with it's functionality back, unless I think of a different way; but it seems that this would kill a couple birds with one stone i.e. formatting regardless of backed (file/stdout) and overwriting characters.
Bumps [tracing-appender](https://github.com/tokio-rs/tracing) from 0.2.3 to 0.2.4. - [Release notes](https://github.com/tokio-rs/tracing/releases) - [Commits](tokio-rs/tracing@tracing-appender-0.2.3...tracing-appender-0.2.4) --- updated-dependencies: - dependency-name: tracing-appender dependency-version: 0.2.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [tracing-subscriber](https://github.com/tokio-rs/tracing) from 0.3.20 to 0.3.21. - [Release notes](https://github.com/tokio-rs/tracing/releases) - [Commits](tokio-rs/tracing@tracing-subscriber-0.3.20...tracing-subscriber-0.3.21) --- updated-dependencies: - dependency-name: tracing-subscriber dependency-version: 0.3.21 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
changelog: ignore
changelog: ignore
Most of the underlying parsing/handling logic is completed, if not all.
There are a _few_ things I'm not quite sure about yet but I'm waiting to
see how it plays out after hooking up the rendering and being able to
see results/behavior.
All of the tests are passing, added a few more to verify logic in a few
spots. Cursor movement, color parsing, line overwriting and span management/
splitting all seem to be functioning correctly.
I believe the majority of what is left to do is directly related to the
actual rendering to the terminal. Along with figuring out double bufferring
and how to write it up to enable tui 'popups' (like a menu for session
management). Some things to keep in mind:
- Still need to track the `ScreenBuffer::view_start` with lines
- After lines are 'out of view', they should be truncated (trim
whitespace/empty cells) since they won't be written to anymore and
truncating will help with their memory size in the VecDeque
- While hooking up the rendering logic, `serial_actor/*.rs` should be
re-evaluated and refactored - currently looks like a mess. Would be
nice to split the task functions up where possible to avoid big
Future structs on heap.
- Switch to crossbeam for channels? Not sure how they play w/ Tokio
- Same with parking_lot and mutexes or maybe RwLock where fitting.
Been awhile since I looked at that code so I don't remember what it
is or isn't using.
- Should spend a day pumping out tests for all of the core
functionality to get a baseline
- Hook up `test-sericom` so that testing logic can be a little more
real-world and longer-running 'sessions' that can be programatically
run and verified.
Lastly, this won't be relevant for a while yet, but still something to
keep thinking about: how and where can `sericom-core` be split up behind
cargo features to enable usage across different frontends i.e. CLI ||
GUI
changelog: ignore
Bumps [tracing](https://github.com/tokio-rs/tracing) from 0.1.41 to 0.1.43. - [Release notes](https://github.com/tokio-rs/tracing/releases) - [Commits](tokio-rs/tracing@tracing-0.1.41...tracing-0.1.43) --- updated-dependencies: - dependency-name: tracing dependency-version: 0.1.43 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [tracing-subscriber](https://github.com/tokio-rs/tracing) from 0.3.20 to 0.3.22. - [Release notes](https://github.com/tokio-rs/tracing/releases) - [Commits](tokio-rs/tracing@tracing-subscriber-0.3.20...tracing-subscriber-0.3.22) --- updated-dependencies: - dependency-name: tracing-subscriber dependency-version: 0.3.22 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Committing so that I can open a new branch for the REPL CLI feature for sericom changelog: ignore
changelog: ignore
Merging the REPL feature branch to start figuring out how `SessionManager` will play with the REPL CLI changelog: ignore
Committing so that I can open a new branch for the REPL CLI feature for sericom changelog: ignore (sericom-core)
Debug events should be emitted via tracing events within sericom-core and captured by the consumer i.e. sericom and handled as seen fit. In sericom's case, it will simply use `tracing-appender` to capture the trace events to a file. changelog: ignore
Decided to change `Cell::character` from `char` to `u8` because `Cell`s are only dynamically constructed from the `ByteParser` and all text is verified to be ascii. This means that it would be no problem to store as `u8` and simply cast the `u8` to a char when printing/writing if needed. Switching from `char` to `u8` reduces the size and alignment of `Cell`, and since there will be _loads_ of `Cell`s in memory, I thought this was a valid change. When using `char` the size of `Cell` is 8 and alignment is `4`, when using `u8`, the size is `2` and alignment `1`.
Changed usize fields to u32 to half field size on 64 bit systems since there are guards/logic in place where the values really should never be > usize::MAX
Implemented the basic foundation/structure for the SessionManager. I am pretty satisfied with the data structure of SessionManager and implemented the basic methods for managing sessions from the SessionManager. changelog: ignore
Reformatting sericom's CLI API for this new REPL implementation. Working towards refactoring the starting/stopping of a session. changelog: ignore
Got rid of the `tokio::select!` in `sericom::main()` and shuffled state to be held in `sericom::run_repl()` for easier usability - works for now. Also removed some match arms of `map_miette!` macro. The rest is just updating the tests to reflect these changes.
Fixed pretty much just to work. changelog: ignore
- Changed `exit-script` cli flag to `script` - Refactored `sericom::init_tracing()` so that debug events can be configured via `RUST_LOG` - Updated CLI API docs - Added `kill` subcommand to kill/close sessions - Added top-level `--color` flag for baseline of implementing colored/plain output depending on whether writing to stdout or pipe
changelog: ignore
Cleaned up a lot of code in `sericom-core` as well.
Lots of changes all around the codebase, would have been nice to make smaller commits but undoing-rewriting led me on a wild goose chase all over to polish a few things up and refactor to fit this new system. Still mutalating 'sericom/src/main.rs' as I play with the REPL and notice things, or as needed for interop with sericom_core. Tracing/debugging/logging is still a WIP, trying to find a flow/system that I like, but a lot of changes in files from this commit were playing with the tracing/debugging. *File-writing:* The file-writing task seems to be where it was before this refactor. A nice improvement that came along with this refactor is that it will now write lines that have already been parsed - directly from the ScreenBuffer. This eliminates just writing whatever the session received (escape sequence galore). *Testing:* Wired up the testing harness to simulate a serial connection, important to note that this only works on Linux and don't plan on implementing it for Windows because I hate Windows. See 'sericom-core/src/session/handle.rs' for how to use the harness.
Owner
Author
simply not planning on implementing windows compatibility for this testing feature unless someone actually needs it |
changelog: ignore
changelog: ignore
changelog: ignore
Finished the implementation of the new version of Parsing/Driving. It is relatively the same, main difference is the use of `#[repr(u8)]` enums for matching on incoming bytes/ansii codes. Intent was to make readability easier and ended up making a few things easier/nicer too. Have yet to really do much testing, I know that the ORIGIN and cursor positions all over the codebase are 0-based, and they should really be 1-based so that will need to be fixed. Briefly made a test for the new ByteParser and it seems to be working fine. Really just need to start writing tests now to try and find any broken/unexpected behaviors.
Converted the Position<TermPos> to be 1-based (1,1) is ORIGIN since that is what control sequences expect. Tested a majority of the new implementation and fixed things as a result. Still a few things to test like scrolling and ensuring the view into the ScreenBuffer's buffer is up-to-date. Next thing on the agenda is to wire up the interactive session. changelog: ignore
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As sericom sits today, it's rendering logic gets the job done, but there are a few bugs that occur. Most bugs seem to revolve around how the cursor movement/escape codes are handled.
This PR serves to re-write the rendering system/logic in sericom-core and properly implement the handling of terminal escape sequences. The idea is that if it properly handles terminal escape sequences then there should be less bugs related to the rendering. The goal is to create a solid foundation to make implementing future features smoother and decoupling some of the convoluted system structures so that it shouldn't be a pita to fix something regardless of where it lies in the rendering system/process.