Skip to content

refactor(rs): json struct#21

Merged
mibli merged 5 commits intomasterfrom
refactor/json_struct
Jul 4, 2025
Merged

refactor(rs): json struct#21
mibli merged 5 commits intomasterfrom
refactor/json_struct

Conversation

@mibli
Copy link
Owner

@mibli mibli commented Jul 4, 2025

Switched to serde_json struct parsing.

Now we have a Node struct that requires most of the fields, which are present in i3 tree, to be present in source json.
The main benefit is much clearer and simpler logic, with a bunch of unwrapping removed.
The downside though is that test jsons growed substantialy, so I've decided to move the jsons to their own files. This however allowed to verify them with check-jsonschema, which double checks that we're not testing on incorrect jsons.

I'm pretty confident in the result. Performance gains are minimal if any (under 10%). XCB still remains the performance leader. However it's definietely a move in the right direction as far as compliance with common practices go. And honestly. Not losing on performance is already a pretty big win.

This will close #20

@mibli mibli changed the title Refactor/json struct refactor(rs): json struct Jul 4, 2025
@mibli mibli requested a review from Copilot July 4, 2025 20:03
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Refactors the i3 backend to use a Node struct with serde_json deserialization, replacing the old manual Value–based logic and consolidating JSON fixtures with schema validation.

  • Introduced backend/i3/json.rs with Node and methods for tabs/visibility.
  • Removed the old compass.rs and updated Backend to parse directly into Node.
  • Moved test JSON fixtures into rust/jsons/, added node.jsonschema, and wired schema checks into the Makefile.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
rust/src/types/rect.rs Added conditional Deserialize derive for Rect when i3 feature is enabled
rust/src/backend/i3/mod.rs Renamed compass module to json
rust/src/backend/i3/json.rs New Node struct with serde parsing, tree-traversal methods, and tests
rust/src/backend/i3/compass.rs Removed old manual JSON parsing implementation
rust/src/backend/i3/backend.rs Updated to parse i3 tree into Node and use its methods
rust/jsons/*.json Moved test fixtures out to dedicated files
rust/jsons/node.jsonschema Added JSON schema to validate test fixtures
rust/Makefile Added schema target to run check-jsonschema before tests
rust/Cargo.toml.in Added optional serde dependency under the i3 feature
Comments suppressed due to low confidence (1)

rust/src/types/rect.rs:11

  • [nitpick] You can merge these two derives into one attribute: #[derive(Deserialize, Debug, Clone, Copy, PartialEq, Eq)] for clearer code.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]

@mibli mibli merged commit d7cdb01 into master Jul 4, 2025
9 checks passed
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.

refactor(rs): Use JSON structs for i3 backend

2 participants