[MVP] Integrated Terminal #4649

andreytkachenko · 2022-11-08T10:12:09Z

Minimal implementation of integrated terminal based on alacritty_terminal crate.
Checklist:

Also nice to have later:

split and tab layouting
ssh tty
text motions support

archseer · 2022-11-08T10:40:01Z

helix-view/Cargo.toml

@@ -42,6 +43,7 @@ toml = "0.5"
 log = "~0.4"

 which = "4.2"
+alacritty_terminal = "0.17.0"


Can you add this dependency only in the helix-vte crate, then re-export it via pub use alacritty_terminal there? That way it's not included several times

I really don't think helix-vte should know anything about alacritty_terminal. helix-vte it is about process PTY or SSH.

Hmm okay. How about splitting helix-view/src/terminal.rs etc into a helix-terminal crate? I'd like to keep the least amount of code possible in helix-view so it's more separated from the rest of -view

Only concern is - the name (helix-terminal) is confusing with helix-term

helix-term/src/ui/terminal.rs

helix-term/src/application.rs

helix-view/src/handlers/vte.rs

helix-vte/src/pty/command.rs

sigmaSd · 2022-11-09T01:01:40Z

Was wezterm https://github.com/wez/wezterm/tree/main/term considered ? The advantage is the number of protocols it implements It has sixel iterm2 and kitty support and even more stuff already implemented.

Nvim terminal is implementing kitty protocol progressively and helix itself supports it since its using crossterm

archseer · 2022-11-09T01:24:17Z

Good call, it should also be a bit more lightweight since you need to bring your own pty provider

andreytkachenko · 2022-11-09T08:14:38Z

Was wezterm https://github.com/wez/wezterm/tree/main/term considered ? The advantage is the number of protocols it implements It has sixel iterm2 and kitty support and even more stuff already implemented.

Nvim terminal is implementing kitty protocol progressively and helix itself supports it since its using crossterm

Hi, yes sure, but I decided that pulling whole wezterm repo only for term crate is too heavy and using alacritty_terminal instead. Since they push the term crate into crates.io it would be nice to switch to it later.

archseer · 2022-11-10T09:26:32Z

helix-vte/src/lib.rs

+impl Default for PtySpawnConfig {
+    fn default() -> Self {
+        Self {
+            command: "/usr/bin/zsh".to_string(),


Let's pass this in and use editor.config.shell

helix/helix-view/src/editor.rs

Lines 124 to 125 in 5835d0b

/// Shell to use for shell commands. Defaults to ["cmd", "/C"] on Windows and ["sh", "-c"] otherwise.

pub shell: Vec<String>,

I think we shouldn't use that option, because by default it is just /bin/sh and that is good for simple cases like pipe through some program. But when we starting the terminal we usually would like to see default system shell program. For now I am setting it value from SHELL env variable, but later it would be better make some terminal section in main configuration file.

dgkf · 2022-11-13T15:52:31Z

helix-vte/src/error.rs

+    #[error("IO Error: {0}")]
+    IoError(#[from] std::io::Error),
+
+    #[error("Termina Not Found: {0}")]


Suggested change

#[error("Termina Not Found: {0}")]

#[error("Terminal Not Found: {0}")]

Totally benign typo - just happened to be reading through the code and spotted it.

xJonathanLEI · 2022-11-16T03:26:50Z

FWIW Zellij uses wezterm.

archseer · 2022-11-16T05:08:52Z

It actually also uses alacritty. https://github.com/zellij-org/zellij/search?q=alacritty https://github.com/zellij-org/zellij/search?q=wezterm

kazimir-malevich · 2023-05-10T05:01:06Z

Is the consensus to include a built in terminal as opposed to a multiplexer? If so what needs to be done?

xprnio · 2023-07-08T06:04:12Z

Is the consensus to include a built in terminal as opposed to a multiplexer? If so what needs to be done?

Random question as an outsider - is there a difference to the end-user?
AFAIK if a terminal was included instead of a multiplexer, wouldn't you be able to start a multiplexer (eg. tmux) inside the terminal? And wouldn't including a multiplexer basically accomplish the same, but without requiring the user to do the multiplexer hop?

archseer · 2023-07-08T17:57:51Z

There is no difference in our case. A built-in in terminal would automatically be a multiplexer since we'd be multiplexing it in a view split

kazimir-malevich · 2023-07-09T02:17:34Z

There has to be way to integrate the warp terminal with Helix. Can't we just split the windows and have keybindings to change focus?

andreytkachenko · 2023-07-11T14:54:51Z

@kazimir-malevich I suspect it wouldn't be integrated due to license incompatibility

Warp (https://warp.dev) is currently closed-source.

xprnio · 2023-07-12T15:38:34Z

Not to mention it's (from the looks of it) MacOS-only

(removed the @ as to not ping people for this irrelevant comment)

kazimir-malevich I suspect it wouldn't be integrated due to license incompatibility

Warp (https://warp.dev) is currently closed-source.

nyabinary · 2023-09-17T00:34:15Z

What's the status on this PR?

xJonathanLEI · 2023-09-17T04:06:28Z

The status is Open.

andreytkachenko · 2023-09-17T07:17:34Z

What's the status on this PR?

archseer reviewed Nov 8, 2022

View reviewed changes

helix-term/src/ui/terminal.rs Outdated Show resolved Hide resolved

archseer reviewed Nov 8, 2022

View reviewed changes

helix-term/src/application.rs Outdated Show resolved Hide resolved

archseer reviewed Nov 8, 2022

View reviewed changes

helix-view/src/handlers/vte.rs Outdated Show resolved Hide resolved

archseer reviewed Nov 8, 2022

View reviewed changes

helix-vte/src/pty/command.rs Outdated Show resolved Hide resolved

kirawi added A-helix-term Area: Helix term improvements S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 8, 2022

the-mikedavis linked an issue Nov 8, 2022 that may be closed by this pull request

[Proposal] Buitin support for Integrated Terminal #1976

Open

andreytkachenko force-pushed the terminal branch from e14e136 to 9d50b02 Compare November 8, 2022 19:26

andreytkachenko force-pushed the terminal branch 5 times, most recently from ea69dec to 5835d0b Compare November 10, 2022 06:12

archseer reviewed Nov 10, 2022

View reviewed changes

andreytkachenko force-pushed the terminal branch 2 times, most recently from e01c2fb to e79ebfc Compare November 10, 2022 13:48

Integrated Terminal

450c9da

andreytkachenko force-pushed the terminal branch from e79ebfc to 450c9da Compare November 10, 2022 13:53

dgkf reviewed Nov 13, 2022

View reviewed changes

archseer mentioned this pull request Dec 8, 2022

[Proposal] Buitin support for Integrated Terminal #1976

Open

This was referenced Dec 8, 2022

feat(lsp): code lenses #5063

Draft

[feature] Natively testrunner #5168

Open

kirawi added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 22, 2022

dradtke mentioned this pull request Feb 14, 2023

Add test-running capability #5993

Closed

pascalkuthe mentioned this pull request Jun 23, 2023

Helix editor should re-render after shell command is exited #4099

Closed

andreytkachenko closed this Sep 17, 2023

andreytkachenko reopened this Sep 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MVP] Integrated Terminal #4649

[MVP] Integrated Terminal #4649

andreytkachenko commented Nov 8, 2022 •

edited

Loading

archseer Nov 8, 2022

andreytkachenko Nov 10, 2022

archseer Nov 10, 2022

andreytkachenko Nov 10, 2022

sigmaSd commented Nov 9, 2022

archseer commented Nov 9, 2022

andreytkachenko commented Nov 9, 2022

archseer Nov 10, 2022

andreytkachenko Nov 10, 2022

dgkf Nov 13, 2022

xJonathanLEI commented Nov 16, 2022

archseer commented Nov 16, 2022

kazimir-malevich commented May 10, 2023

xprnio commented Jul 8, 2023

archseer commented Jul 8, 2023

kazimir-malevich commented Jul 9, 2023

andreytkachenko commented Jul 11, 2023

xprnio commented Jul 12, 2023

nyabinary commented Sep 17, 2023

xJonathanLEI commented Sep 17, 2023

andreytkachenko commented Sep 17, 2023

	/// Shell to use for shell commands. Defaults to ["cmd", "/C"] on Windows and ["sh", "-c"] otherwise.
	pub shell: Vec<String>,

	#[error("Termina Not Found: {0}")]
	#[error("Terminal Not Found: {0}")]

[MVP] Integrated Terminal #4649

Are you sure you want to change the base?

[MVP] Integrated Terminal #4649

Conversation

andreytkachenko commented Nov 8, 2022 • edited Loading

archseer Nov 8, 2022

Choose a reason for hiding this comment

andreytkachenko Nov 10, 2022

Choose a reason for hiding this comment

archseer Nov 10, 2022

Choose a reason for hiding this comment

andreytkachenko Nov 10, 2022

Choose a reason for hiding this comment

sigmaSd commented Nov 9, 2022

archseer commented Nov 9, 2022

andreytkachenko commented Nov 9, 2022

archseer Nov 10, 2022

Choose a reason for hiding this comment

andreytkachenko Nov 10, 2022

Choose a reason for hiding this comment

dgkf Nov 13, 2022

Choose a reason for hiding this comment

xJonathanLEI commented Nov 16, 2022

archseer commented Nov 16, 2022

kazimir-malevich commented May 10, 2023

xprnio commented Jul 8, 2023

archseer commented Jul 8, 2023

kazimir-malevich commented Jul 9, 2023

andreytkachenko commented Jul 11, 2023

xprnio commented Jul 12, 2023

nyabinary commented Sep 17, 2023

xJonathanLEI commented Sep 17, 2023

andreytkachenko commented Sep 17, 2023

andreytkachenko commented Nov 8, 2022 •

edited

Loading