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.

Sign up for GitHub

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

Jump to bottom

Code health: consider splitting Pane class into leaf and parent classes #3248

Open
mcpiroman opened this issue Oct 18, 2019 · 4 comments
Open

Code health: consider splitting Pane class into leaf and parent classes #3248

mcpiroman opened this issue Oct 18, 2019 · 4 comments
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Milestone
Backlog

Comments

@mcpiroman
Copy link
Contributor

Idea: Split Pane class into LeafPane and ParentPane, both inheriting from abstract base Pane.

Currently Pane class has many members that are used only in leaf or only in parent mode. IMHO proposed splitting would make the code much cleaner and safer (e.g. from accidentally accessing a terminal when we're not a leaf). There is a long way ahead of panes so this might be worthwhile.
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Oct 18, 2019
@oising oising added Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. and removed Needs-Tag-Fix Doesn't match tag requirements labels Oct 18, 2019
@DHowett-MSFT DHowett-MSFT added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Oct 21, 2019
@ghost ghost added the Needs-Tag-Fix Doesn't match tag requirements label Oct 21, 2019
@DHowett-MSFT
Copy link
Contributor

This one really needs @zadjii-msft's input. Thanks!

@DHowett-MSFT DHowett-MSFT added this to the Terminal Backlog milestone Oct 21, 2019
@DHowett-MSFT DHowett-MSFT added the Issue-Task It's a feature request, but it doesn't really need a major design. label Oct 21, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 21, 2019
@DHowett-MSFT DHowett-MSFT self-assigned this Oct 21, 2019
@zadjii-msft
Copy link
Member

This seems to make sense to me. I can't think of a good reason why we wouldn't want it to be that way TBH.

@mcpiroman
Copy link
Contributor Author

And to support non-terminal panes (#997), maybe also make LeafPane abstract and then inherit from it in TerminalPane and alike. What do you think?

@zadjii-msft
Copy link
Member

So as far as #997 is concerned, I actually prototyped a bunch of that back in July, but never got it polished enough to release. YOu can see where I was going with it in this branch:
https://github.com/microsoft/terminal/compare/dev/migrie/f/non-terminal-panes

There's a good chance that reafactoring into parent/leaf panes would make this process even easier, but I was considering just adding an entire other abstraction layer between the Pane and the content anyways.

@DHowett-MSFT DHowett-MSFT removed their assignment Nov 6, 2019
@mcpiroman mcpiroman mentioned this issue Dec 27, 2019
Closed
5 tasks
@ghost ghost added the In-PR This issue has a related PR label Dec 27, 2019
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
@ghost ghost removed the In-PR This issue has a related PR label Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-CodeHealth Issues related to code cleanliness, linting, rules, warnings, errors, static analysis, etc. Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

Split Pane class into LeafPane and ParentPane + small fixes mcpiroman/terminal
4 participants
@oising @DHowett-MSFT @zadjii-msft @mcpiroman