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

Spawning editors #171

Closed
antonmedv opened this issue Dec 13, 2021 · 21 comments · Fixed by #237
Closed

Spawning editors #171

antonmedv opened this issue Dec 13, 2021 · 21 comments · Fixed by #237
Labels
enhancement New feature or request

Comments

@antonmedv
Copy link

Hi,

Thanks for the awesome project!

Question: how to properly start subcommand with reuse of Stdout/Stdin?

I'm doing it like so:
https://github.com/antonmedv/llama/blob/58c042eb06a5a661a1388beb4ac09cedf2764ad0/main.go#L183-L190

func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	if m.editMode {
		return m, nil
	}
				cmd := exec.Command(lookup([]string{"LLAMA_EDITOR", "EDITOR"}, "less"), filepath.Join(m.path, m.cursorFileName()))
				cmd.Stdin = os.Stdin
				cmd.Stdout = os.Stdout
				// Note: no Stderr as redirect `llama 2> /tmp/path` can be used.
				m.editMode = true
				_ = cmd.Run()
				m.editMode = false
				return m, tea.HideCursor

But what to do if a redraw is needed after cmd exits? And is there a better way to halt the program until cmd is exited?

Thanks.

Originally posted by @antonmedv in #170

@meowgorithm
Copy link
Member

meowgorithm commented Dec 13, 2021

Hi! Your code's pretty close. As a rule, you'll want to move all long running code into a tea.Cmd, which runs in a goroutine internally. tea.Cmds return a message when finished, which triggers an update and in, a redraw. So I'd do something roughly like this:

type editingFinished struct{}

func openEditor(app string, args ...string) tea.Cmd {
	return func() tea.Msg {
		cmd := exec.Command(app, args...)
		cmd.Stdin = os.Stdin
		cmd.Stdout = os.Stdout
		_ = cmd.Run() // blocks until finished
		return editingFinished{}
	}
}

func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
	if m.editMode {
		return m, nil
	}

	switch msg := msg.(type) {
	case tea.KeyMsg:
		switch msg.String() {
		case "e":
			m.editMode = true
			return m, tea.Batch(
				tea.HideCursor,
				openEditor(lookup([]string{"LLAMA_EDITOR", "EDITOR"}, "less"), filepath.Join(m.path, m.cursorFileName())),
			)
		}
	case editingFinished:
		m.editMode = false
		return m, nil
	}

	// ...
}

Really nice work on llama, by the way. We actually added tea.HideCursor for exactly your use case.

And by all means, let us know if you have any more questions.

@antonmedv
Copy link
Author

Thanks a lot! 🔥

@antonmedv antonmedv reopened this Dec 16, 2021
@antonmedv
Copy link
Author

For some reason, this approach does not work. And from example, vim and tea outputs get messed up. Can't figure out why. Trying to implement it in llama.

@meowgorithm
Copy link
Member

Saw your other issue. It's probably unrelated to the above approach and in the end (once we figure it out) you'll mostly likely need to move stuff back into a command (illustrated above) in order a trigger a repaint.

@mistakenelf
Copy link

I also have the same issue when trying to open a file to edit via vim and then back into the app, the output gets messed up even after triggering a command to cause a repaint

@antonmedv
Copy link
Author

Probably, the right approach will be to use PTY.

@meowgorithm
Copy link
Member

meowgorithm commented Dec 18, 2021

I've looked into this one a little bit. No conclusions yet, but my hunch says we'll need to introduce some functionality into Bubble Tea (likely by way of Cmds) to do this properly. A few notes so far:

  1. Opening Vim by blocking in Update is reliable for me so far. If Bubble Tea is using the altscreen the Bubble Tea view does not repaint after exiting Vim, even when returning a command like tea.HideCursor.
  2. Opening Vim via a tea.Cmd results in very poor performance in Vim, an occasional input errors. For this approach to work Bubble Tea will likely need some additional functionality to effectively pause portions of the runtime. Opening dedicated TTYs for input in both Bubble Tea and Vim did not help.
  3. When using the alt screen in Bubble Tea, redraws did not occur after exiting Vim, even when requesting to re-enter via tea.EnterAltScreen.

Here's an example program I've been testing with.

@knipferrc and @antonmedv: if it's convenient, I'd love to see a screenshot of the garbled output you're describing after closing Vim. I'm not able to reproduce that on my end yet.

@mistakenelf
Copy link

Sure thing. Here is what I see once I start Vim in a command

image

@antonmedv
Copy link
Author

I tried to integrate github.com/creack/pty but this is quite difficult, we can't just io.Copy, and need to get part of UI as a string.

@bensadeh
Copy link
Contributor

Hi everyone 👋,

Just wanted to chime in and hopefully give some useful pointers.

This discussion is quite similar to one in tcell, another TUI framework which is also written in Go and had the same issue with running subcommands.

Long story short: tcell had a similar issue which was eventually fixed here. I wish I could be more helpful than just refer to the ticket (most of the implementation goes over my head), but I hope that the implementation can be useful for Bubble Tea.

(Btw, Bubble Tea is really great, I have been eagerly awaiting for this subcommand feature to see of I can replace the current TUI in clx)

@mritd
Copy link

mritd commented Jan 10, 2022

I successfully opened vim according to the example, but after vim exited, I received a keymsg:

image

image

It looks like this button appears in the standard input after the execution of vim is complete; I don't know what caused it.

@meowgorithm Do you have any suggestions?

@meowgorithm
Copy link
Member

@mritd That appears to be an open brace ([, i.e. rune(91)) which I'm guessing is part of a broken ANSI sequence. Hard to say where it's coming from without more context, however if it's something you're seeing consistently you could add some logic to ignore it after Vim exits.

@meowgorithm meowgorithm changed the title Running sub commands Spawning Editors Jan 10, 2022
@meowgorithm
Copy link
Member

meowgorithm commented Jan 10, 2022

I also wanted to provide some updates on this issue. We're narrowing this ticket to be specific to spawning editors from within Bubble Tea (versus any sub-process), because most other processes (both blocking and non-blocking) can be currently handled fine in a tea.Cmd.

Anyway, this is something we'd like to officially support in Bubble Tea. There are essentially two things to tackle:

  1. Spawning editors that block in the Terminal, such Vim. This is an easier case to handle because editors that live in the terminal will block until finished, so it's mostly just a matter of providing what will probably be a special tea.Cmd to handle editor spawning and cleanup.
  2. Spawning editors that don't block, such as opening a file in VSCode. This is a more difficult case because it requires file-watching, which comes with a lot of pitfalls, particularly os-specific ones.

In the meantime, for those interested in spawning in-terminal editors like Vim (@antonmedv, @mritd, @knipferrc) I recommend taking a look at edist, which seems to have a working, albeit hacky, solution. In particular this file illustrates the implementation and there are some detail on some pitfalls in PR#1.

@meowgorithm meowgorithm added the enhancement New feature or request label Jan 10, 2022
@meowgorithm meowgorithm changed the title Spawning Editors Spawning editors Jan 10, 2022
@JudgeGregg
Copy link

JudgeGregg commented Feb 3, 2022

Hello, a few elements that might be helpful :)

I tried to run your example program in several environments (all Linux based):

With kitty (https://sw.kovidgoyal.net/kitty/)

  1. opening vi:
  • some keys have no effect (are "swallowed"). E.g. pressing 'i' twice doesn't display anything. Only happens when vi has just been opened.
  • doesn't crash
  • cursor OK
  1. opening neovim (nvim):
  • some keys swallowed but harder to reproduce than in vi
  • regularly crashes with
    Error running program: read /dev/stdin: resource temporarily unavailable exit status 1 (not always but roughly 1 time out of 3)
  • cursor OK
  1. opening vim:
  • some keys have no effect (same as vi)
  • no crashes
  • cursor OK

With neovim in kitty (opening a term in neovim with :terminal, then running the example, so we have vi, vim or nvim in nvim)

  1. opening vi:
  • some keys have no effect (same as vi)
  • no crashes
  • no cursor displayed in vi, cursor reappears when back to the shell
  1. opening neovim:
  • some keys swallowed but harder to reproduce than in vi
  • regular crashes related to stdin
  • cursor OK
  1. opening vim:
  • some keys swallowed but harder to reproduce than in vi
  • no crashes related to stdin
  • cursor OK

It seems that the "swallowed" keys are interpreted by bubbletea. E.g. typing q at as your first key will sometimes lead to exiting the go program as soon as vi exits.
I tried the same experiments with xterm and the results were consistent with those above.
All these tests have been run with "vim/nvim -u NONE" to limit the influence of my vim config.

Thank you for your work on the charm projects, they are very fun to use :)

@muesli
Copy link
Member

muesli commented Feb 21, 2022

I've just posted a draft PR that adds program.CancelInput and program.RestoreInput, as well as a little example demonstrating how to launch an editor: #237

While the PR is not ready to merge just yet, I'd love to hear your feedback: does it fix the swallowed keys issue and stdin crashes/errors for you? (it does for me)

@JudgeGregg
Copy link

JudgeGregg commented Feb 22, 2022

Yes, using the exec-cancelreader branch does fix all crashes and swallowed keys, in each of the situations described above. Thanks a lot :)

@muesli
Copy link
Member

muesli commented Feb 25, 2022

Yes, using the exec-cancelreader branch does fix all crashes and swallowed keys, in each of the situations described above.

Thanks for the feedback! I'll clean this PR up a bit and we'll merge this soon!

@taigrr
Copy link
Contributor

taigrr commented Mar 24, 2022

@muesli has there been any progress on this front? I'd love to use bubbletea in a project I'm speccing out right now, but this would be a blocking issue. Thanks!

@muesli
Copy link
Member

muesli commented Mar 24, 2022

@muesli has there been any progress on this front? I'd love to use bubbletea in a project I'm speccing out right now, but this would be a blocking issue. Thanks!

Yep, we're actively working on this, see PR #237.

@meowgorithm
Copy link
Member

Hey everyone! This is now available in master and will be available in the next release. For an example on how to use this new feature have a look at this example.

@antonmedv
Copy link
Author

Thanks a lot for your awesome library!

lorenzo-milicia pushed a commit to lorenzo-milicia/bubbles that referenced this issue Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
8 participants