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

Refactor Pane #3764

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Refactor Pane #3764

wants to merge 2 commits into from

Conversation

a-usr
Copy link

@a-usr a-usr commented Nov 12, 2024

Refactor Pane somewhat according to
// FIXME: Use a struct that has a pane_type enum, to reduce all of the duplication

As of now I've added a enum (Pane) which holds either a TerminalPane, or a PluginPane, and derefs to the PaneTrait (previously just Pane) trait.

Things I need to know:

  • What methods from the PaneTrait trait to move to PluginPane and TerminalPane respectively
  • Wether zellij-server/src/panes/mod.rs is an acceptable (new) home for both the Pane enum and the PaneTrait trait (PaneTrait seems kind of out of place in zellij-server/src/tab/mod.rs)
  • Wether I should rename PaneTrait to something better (and what)

Other Ideas and suggestions are more than welcome

Regarding the methods that should be moved

Here is a quick list of methods that I think might be better off outside of PaneTrait:

Methods that I might be better off in TerminalPane

fn exited(&self) -> bool {

fn exit_status(&self) -> Option<i32> {

fn rerun(&mut self) -> Option<RunCommand> {

Methods that might be better off in PluginPane

fn update_loading_indication(&mut self, _loading_indication: LoadingIndication) {} // only relevant for plugins
fn start_loading_indication(&mut self, _loading_indication: LoadingIndication) {} // only relevant for plugins
fn progress_animation_offset(&mut self) {} // only relevant for plugins

Q&A

Q: Why make this Draft instead of just coding away?

A: Because I prefer gathering info and tips before doing a somewhat huge amount of changes over having to scrap 90% of my code because it doesnt conform to the general vision of the Maintainers

Q: Why this?

A: Because Im currently working on another (smaller) change, and I dont want to have to write Spagetti Code to make it work. And Im sure the Maintainer(s) dont want me to either.

@a-usr
Copy link
Author

a-usr commented Nov 12, 2024

Im just gonna let this sit here for the next week or so, and hope the people that need to see this do.

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.

1 participant