Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(frame)!: Remove generic Backend parameter #530

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

joshka
Copy link
Member

@joshka joshka commented Sep 22, 2023

The purpose of this change is to simplify UI code that uses the Frame
type. E.g.:

fn draw<B: Backend>(frame: &mut Frame<B>) {
    // ...
}

Frame was generic over Backend because it stored a reference to the
terminal in the field. Instead it now directly stores the viewport area
and current buffer. These are provided at creation time and are valid
for the duration of the frame.

BREAKING CHANGE: Frame is no longer generic over Backend. Code that
accepted a Frame will now need to accept a Frame.

The purpose of this change is to simplify UI code that uses the Frame
type. E.g.:

```
fn draw<B: Backend>(frame: &mut Frame<B>) {
    // ...
}
```

Frame was generic over Backend because it stored a reference to the
terminal in the field. Instead it now directly stores the viewport area
and current buffer. These are provided at creation time and are valid
for the duration of the frame.

BREAKING CHANGE: Frame is no longer generic over Backend. Code that
accepted a Frame<Backend> will now need to accept a Frame.
@github-actions github-actions bot added the Type: Breaking Change This change will cause application code to break and must be noted in the breaking changes docs etc. label Sep 22, 2023
@codecov
Copy link

codecov bot commented Sep 22, 2023

Codecov Report

Merging #530 (93334b9) into main (d67fa2c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #530   +/-   ##
=======================================
  Coverage   90.06%   90.06%           
=======================================
  Files          40       40           
  Lines       11267    11268    +1     
=======================================
+ Hits        10148    10149    +1     
  Misses       1119     1119           
Files Changed Coverage Δ
src/layout.rs 98.38% <ø> (ø)
src/widgets/block.rs 93.44% <ø> (ø)
src/widgets/clear.rs 52.00% <ø> (ø)
src/widgets/scrollbar.rs 89.76% <ø> (ø)
src/terminal.rs 58.46% <100.00%> (+0.16%) ⬆️

@kdheepak
Copy link
Collaborator

I haven't checked all the tests but what happens if a terminal resize occurs? Does this handle that?

@joshka
Copy link
Member Author

joshka commented Sep 22, 2023

I haven't checked all the tests but what happens if a terminal resize occurs? Does this handle that?

Frames are only created by terminal.get_frame(&mut self) which borrows terminal mutably and prevents the resize method from being called. The viewport and buffer size can only be changed by something that can mutate the terminal. The following does not compile:

let backend = TestBackend::new(10, 10);
let mut terminal = Terminal::new(backend).unwrap();
let frame = terminal.get_frame();
terminal.resize(Rect::new(0, 0, 5, 5)).unwrap();
 1  error[E0499]: cannot borrow `terminal` as mutable more than once at a time
    --> src/terminal.rs:687:9
     |
 686 |         let frame = terminal.get_frame();
     |                     -------------------- first mutable borrow occurs here
 687 |         terminal.resize(Rect::new(0, 0, 5, 5)).unwrap();
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ second mutable borrow occurs here
 688 |         assert_eq!(frame.size(), Rect::new(0, 0, 10, 10));
     |                    ------------ first borrow later used here
 
 For more information about this error, try `rustc --explain E0499`.
 error: could not compile `ratatui` (lib test) due to previous error

Copy link
Member

@orhun orhun left a comment

Choose a reason for hiding this comment

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

Breaking but good. We should add "migration" instructions to the final commit message.

@joshka
Copy link
Member Author

joshka commented Sep 26, 2023

Breaking but good. We should add "migration" instructions to the final commit message.

Will do

@joshka joshka merged commit 082cbcb into main Sep 26, 2023
33 checks passed
@joshka joshka deleted the feat-frame-remove-generic branch September 26, 2023 05:30
tonogdlp pushed a commit to tonogdlp/ratatui that referenced this pull request Oct 6, 2023
This change simplifys UI code that uses the Frame type. E.g.:

```rust
fn draw<B: Backend>(frame: &mut Frame<B>) {
    // ...
}
```

Frame was generic over Backend because it stored a reference to the
terminal in the field. Instead it now directly stores the viewport area
and current buffer. These are provided at creation time and are valid
for the duration of the frame.

BREAKING CHANGE: Frame is no longer generic over Backend. Code that
accepted `Frame<Backend>` will now need to accept `Frame`. To migrate
existing code, remove any generic parameters from code that uses an
instance of a Frame. E.g. the above code becomes:

```rust
fn draw(frame: &mut Frame) {
    // ...
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Breaking Change This change will cause application code to break and must be noted in the breaking changes docs etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants