DRAFT Spec for Broadcast Input #9365
Conversation
zadjii-msft
added
Area-Input
This comment has been minimized.
This comment has been minimized.
|
I might come back through here with some future considerations blatantly taken from iTerm2's issue tracker. If they've already got this feature, and have people requesting more functionality, then it's inevitable that people will ask the same of us. Maybe we should design the feature with those in mind? |
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
| - **TODO: FOR DISCUSSION**: Should this disable the tab's | ||
| "broadcastToAllPanes" setting? Or should it leave that alone? |
There was a problem hiding this comment.
I think we should leave "broadcastToAllPanes" alone here. That just makes more sense to me IMO. But I'm curious if anybody disagrees.
| does not affect the other. | ||
|
|
||
| #### Cons | ||
| * I frankly think the `tab`/`pane` interaction can be a little weird. |
There was a problem hiding this comment.
I agree. Like for this scenario...
- enable broadcast input for tab 1
- switch to tab 2
- enable broadcast input for a pane in tab 2
There's valid confusion to be had between the following two behaviors:
- input goes to all of tab 1 and the pane in tab 2
- input only goes to the pane in tab 2
| In the original PR, it was suggested to use some variant of the [accent color] | ||
| to on the borders of panes that are currently receiving broadcasted input. This | ||
| would be a decent visual indicator that they're _not_ the active pane, but they | ||
| are going to receive input. Something a bit like: | ||
|
|
||
|  |
There was a problem hiding this comment.
I'm mildly concerned that about the background color of a terminal being the same as the accent color. But this seems like a pretty straightforward approach that wouldn't be too difficult. I feel like there's a better approach though?
There was a problem hiding this comment.
for the record, I'm suggesting a variant of the accent color for the border, where we're already using SystemAccentColor to indicate the active pane
So it's different, but only in saturation/lightness - the hue is generally the same
| iTerm2 also supports displaying "stripes" in the background of all the panes | ||
| that are being broadcast too. That's certainly another way of indicating this | ||
| feature to the user. I'm not sure how we'd layer it with the background image | ||
| though. |
There was a problem hiding this comment.
Huh. I kinda wish the opposite behavior. We'd put stripes on panes that are not getting input. it's a lot more clear imo. And if you're not sending input to a pane, chances are you're not reading it either?
Ugh, this is where I wish "unfocusedAppearance" would work well with other states. Because how cool would it be if you could customize a terminal that's not getting input... wait... what if we did that anyways? Panes that are not taking input use their "unfocusedAppearance", whereas others use the focused appearance? Would that be too weird? (now I'm rambling)
There was a problem hiding this comment.
FWIW, I hate the stripes, and don't want to bother with those. That seems like an unnecessary extra complication
| * [iterm2#6451], [iterm2#5563]: "Broadcast commands" | ||
| - iTerm2 has an action that lets the user manually clear the terminal-side | ||
| buffer. (This is tracked on the Windows Terminal as [#1882]). It might make | ||
| sense for there to be a mode where some _actions_ are also broadcast to | ||
| panes, not just key strokes. But which actions would those be? Moving the | ||
| selection anchors? Copy doesn't really make sense. Paste _does_ though. | ||
| Maybe the open find dialog / next&prev search match actions? | ||
| - This probably would require it's own spec. |
There was a problem hiding this comment.
I really like this idea. This makes me think a bit more about that idea I had with you forever ago about defining a "context" on an action (in the settings model, not exposed to the user). So like, "context=pane" actions would be totally valid, but "context=tab" would not.
Some preliminary ideas of commands that would work in this way include:
- close pane
- paste text
- send input
- scroll
- change font size
Some weird ones that would still work, but are just weird include:
- resize pane
- find
- keyboard selection
- copy
These are weird because they technically do work. We would just do the command on all of the selected panes, and some panes would silently fail by returning false instead of true. That's how we handle ctrl+c and "copy". If a selection is active, we return true because the action was successfully performed. Otherwise, we return false and just pass ctrl+c to the terminal.
| * **[iterm2#5639]: Broadcast groups**, [iterm2#3372]: Broadcast Input to | ||
| multiple but not all tabs |
There was a problem hiding this comment.
Wouldn't you just be able to toggle broadcast input for a tab? And using proposal 2, it's added to the set?
I was thinking you could even add this as an option in the tab's context menu. And that just adds the icon to the tab.
| the user? If we used the broadcast icon with a number, maybe in the corner | ||
| of the tab? Like [📡: 1]? Can a pane be in multiple broadcast sets at the | ||
| same time? | ||
| - The natural arg idea would be `{ "action": "toggleBroadcastInput", "scope": |
There was a problem hiding this comment.
Oh maybe we want something like group: -1 or something that indicates "create a new group". Cause the initial implementation is going to be "add all the panes in this tab to a new group", and each tab has it's own group. So the action added in #9222 would default to "make a new group".
But the default value for the arg shouldn't be -1, because adding each pane to its own broadcast group is useless
|
From @zljubisic in #9222
|
This comment was marked as outdated.
This comment was marked as outdated.
zadjii-msft
force-pushed
the
dev/migrie/s/2634-broadcast-input
branch
from
f4b9d39
to
d53f870
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I'm officially moving this to the |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
zadjii-msft
added
the
AutoMerge
|
Hello @zadjii-msft! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
|
Since this is a doc and has enough approvals, I'm just merging this manually. |
Resurrection of #9222. Spec draft in #9365. Consensus from community feedback is that the whole of that spec is _nice to have_, but what REALLY matters is just broadcasting to all the panes in a tab. So, in the interest of best serving our community, I'm pushing this out as the initial implementation, before we figure out the rest of design. Regardless of how we choose to implement the rest of the features detailed in the spec, the UX for this part of the feature remains the same. This PR adds a new action: `toggleBroadcastInput`. Performing this action starts broadcasting to all panes in this tab. Keystrokes in one pane will be sent to all panes in the tab. An icon in the tab is used to indicate when this mode is active. Furthermore, the borders of all panes will be highlighted with `SystemAccentColorDark2`/`SystemAccentColorLight2` (depending on the theme), to indicate they're also active. * [x] Closes #2634. - (we should lick a reserved thread for follow-ups) Co-authored-by: Don-Vito khvitaly@gmail.com
⇒ doc link ⇐
Summary of the Pull Request
This is supposed to be a quick and dirty spec to socialize the various different options for Broadcast Input mode with the team. Hopefully we can come up with a big-picture design for the feature, so we can unblock #9222.
Abstract
...
PR Checklist
Detailed Description of the Pull Request / Additional comments
*** read the spec ***