More unused header removal #406
Conversation
|
Some of these shouldn't be removed. My goal is to follow the "include what you use" rule (https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md). For example, |
|
Okay, good point. I'll apply more due diligence as I progress as I delve deeper into dividing "view from app" |
…h, these imports are part of PTYSession's conformance declaration (applying "include what you use")
There was a problem hiding this comment.
Looking better, but still a few issues. I love fixing the headers (there was not very good header hygiene for the first 15 years of development), but it's hard to get right. Let me know what your big picture plan is and I might be able to help point you in the right direction.
| @@ -1,28 +1,23 @@ | |||
| // Implements the model class for a terminal session. | |||
|
|
|||
| #import "Api.pbobjc.h" | |||
There was a problem hiding this comment.
This is used in this file. For example, ITMGetBufferResponse. Anything with the ITM prefix comes from the protobufs. However, since only objects are ever referenced this import could be replaced with forward references like @class ITMGetBufferResponse for each referenced symbol.
| #import "DVR.h" | ||
| #import "iTermAPIHelper.h" | ||
| #import "iTermEchoProbe.h" | ||
| #import "iTermFindDriver.h" | ||
| #import "iTermFileDescriptorClient.h" | ||
| #import "iTermMetalUnavailableReason.h" | ||
| #import "iTermWeakReference.h" | ||
| #import "ITAddressBookMgr.h" |
There was a problem hiding this comment.
This file uses iTermSessionEndAction from this include.
| #import "PTYTask.h" | ||
| #import "PTYTextView.h" | ||
| #import "ProfileModel.h" |
There was a problem hiding this comment.
We use the Profile macro in this file, which is defined in ProfileModel.h
| #import "TextViewWrapper.h" | ||
| #import "TmuxController.h" | ||
| #import "TmuxGateway.h" | ||
| #import "VT100Screen.h" | ||
| #import "VT100ScreenMark.h" | ||
| #import "WindowControllerInterface.h" |
There was a problem hiding this comment.
Can replace this with @interface WindowControllerInterface, which is used below
| @@ -26,7 +26,6 @@ | |||
| */ | |||
|
|
|||
| #import <Cocoa/Cocoa.h> | |||
| #import "iTermFindDriver.h" | |||
There was a problem hiding this comment.
SessionView conforms to the iTermFindDriverDelegate protocol declared in this header
|
Okay, I'll roll these back. The "big picture plan" has evolved since I started. I originally was hoping to separate-out an iTerm "ViewKit" library, which had everything necessary to provide an iTerm2 view without any window dressing or auxiliary view controllers (Preferences, WindowController, etc.), even foregoing Python scripting capabilities, along the lines of "Phase 2" that I brought up in PR #366. As I've dived in deeper, now I'm starting to see how tightly coupled these things are, and am considering just "lumping it all together" like PR #366 tried to do (and what my Phase 1 described). And then, maybe abstract-out the auxiliary view controllers after that. The bulk of my work is currently in a local git repo called "IT2ViewKit" which uses cmake to divide things up into nice layers like "IT2Foundation", "IT2Scripting", and IT2ViewKit". Using cmake is great because it doesn't require I touch iTerm's existing xcode project at all, nor even have to commit to an "iTerm2Lib" xcode project. Cmake can generate projects at will. Maybe, even further down the road, when my project is well underway, I may submit a PR that uses cmake to build everything, including iTerm2.app. A PR that does not require committing even iTerm2.xcodeproj to the repo. But, that's jsut pie-in-the-sky for me right now. |
|
Decoupling things will be a necessary part of making it possible to create a library. I'm fine with using cmake but I don't know how to prevent breaking things as changes are made. I suppose when a new file is added both the xcode project and the makefile will need to be updated together? |
Yeah, pretty much like that. I'm hoping to achieve the goal of making that really easy and understandable when the time comes to present that PR. At the moment, I'm trying to find a clean way to avoid inclusion of iTermAPIHelper in the view library. |
I think any attempt at cleanly separating views out from other stuff will require adding a lot of new protocols. Right now the dependency graph is full of cycles because I've been optimizing for velocity over flexibility. I could imagine a world where the API helper is a delegate of the various classes that call directly into it, and then they could tolerate its non-existence. |
|
@gnachman, it's been a few days since my last comment re:Big Picture. Let me fill you in on what I've been to. I paused my work on sorting-out the headers and focused on ensuring that "iTerm2Lib" will be the right solution for my project. The area I'm exploring right now is "how to trick iTerm2Lib into using a pty driver that does a remote connection to a server." You see, I want to use /just/ the text rendering aspects of iTerm2, I'm not interested in opening a local shell, I'm interested in opening a connection to a remote MUD server. I'm thinking I could make a custom pty process/driver that connects to the server and then have iTerm2Lib open that pty session. Problem is, I'm new to dealing with tty/pty dev concepts and am not sure if even such a thing is possible. I'm a little out of my experience zone here. Do you know much about this subject? Do you think I'm taking the right approach of redirecting at the pty level like this? Is there an easier approach? Originally, I started my Savitar v2.0 project using attributed strings sent to an NSTextView. But, I was concerned about performance, so I got attracted to the idea of using an embeddable iTerm2 view instead (yay, Metal!) Besides, iTerm2 also has triggering support, ANSI color code support, and a few other things that I wouldn't have to implement for myself atop of NSTextView (not that I haven't already implemented all of this already in v1.0 of my project.) |
|
I made a rough sketch of how to refactor PTYTask to enable this in the |
I looked over this branch a few days ago, but I didn't see how I might be able to reuse things to do a remote socket. Is such a thing possible? Maybe instead of integrating a socket, I have the session do the remote connect as part of its "shell" script? |
|
Good news, I've landed that change. You need to write a class that implements The most important thing is to vend a file descriptor in your
What do you mean by "'shell' script"? If you mean replacing its |
| @@ -30,7 +30,6 @@ | |||
| #import "iTermMalloc.h" | |||
| #import "iTermSwiftyStringParser.h" | |||
| @@ -30,7 +30,6 @@ | |||
| #import "iTermAdvancedSettingsModel.h" | |||
| #import "iTermScrollAccumulator.h" | |||
| #import "NSView+iTerm.h" | |||
| @@ -1,28 +1,23 @@ | |||
| // Implements the model class for a terminal session. | |||
|
|
|||
| #import "Api.pbobjc.h" | |||
| #import "DVR.h" | |||
| @@ -26,7 +26,6 @@ | |||
| */ | |||
|
|
|||
| #import <Cocoa/Cocoa.h> | |||
| #import "iTermFindDriver.h" | |||
| #import "iTermMetalDriver.h" | |||
| @@ -24,7 +24,6 @@ | |||
| #import "PTYScrollView.h" | |||
| #import "PTYSession.h" | |||
| #import "PTYTab.h" | |||
| #import "PTYTextView.h" | |||
| #import "PTYWindow.h" | |||
| #import "SessionTitleView.h" | |||
|
Ok |
Details
|
Another baby step towards an embeddable iTerm2 library.
Sorry about the redundant commits in this PR, I had a bit of a cockpit error while rebasing my fork's master branch to your master branch. Though I'm an old hand at git and github, I'm kind of new at cross-fork PR submissions.