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

[Bug?] Terminal is not interactive. #1010

Open
npxcomplete opened this issue Jul 17, 2024 · 9 comments
Open

[Bug?] Terminal is not interactive. #1010

npxcomplete opened this issue Jul 17, 2024 · 9 comments

Comments

@npxcomplete
Copy link

Not sure if this is an environment bug or if I'm doing something horribly wrong. I've been working off the example code expecting the below program to let me tab between the 4 input fields and type into text boxes. However, when I start the program up the library paints the terminal and I love how easy that is, but there's no interactivity.

Linux 6.5.x / Ubuntu.

I've tested in ubuntu's default terminal, terminator, and embedded in tmux.

package main

import (
        "fmt"
	"github.com/gdamore/tcell/v2"
	"github.com/rivo/tview"
)

func main() {
	app := tview.NewApplication()

	logGroupSelect := tview.NewDropDown().
		SetLabel("Pick a log group: ").
		SetOptions([]string{"groupA", "groupB"}, func(text string, index int){
		   fmt.Println("Hello")
		})
	logGroupSelect.
		SetTitle("Log Groups").
		SetBorder(true)

	textArea := tview.NewTextArea().
		SetPlaceholder("")
	textArea.SetTitle("Query").SetBorder(true)

	startTime := tview.NewInputField().
		SetLabel("Start Time: ").
		SetFieldWidth(10).
		SetAcceptanceFunc(tview.InputFieldInteger).
		SetDoneFunc(func(key tcell.Key) {
		})

	endTime := tview.NewInputField().
		SetLabel("End Time: ").
		SetFieldWidth(10).
		SetAcceptanceFunc(tview.InputFieldInteger).
		SetDoneFunc(func(key tcell.Key) {
		})

	flex := tview.NewFlex().SetDirection(tview.FlexRow).
		AddItem(logGroupSelect, 0, 3, false).
		AddItem(tview.NewFlex().
			AddItem(startTime, 0, 3, false).
			AddItem(endTime, 0, 3, false),
			0, 3, false).
		AddItem(textArea, 0, 5, false).
		AddItem(tview.NewBox().SetBorder(true).SetTitle("Logs"), 0, 10, false)

	app.SetFocus(logGroupSelect)
	app.SetRoot(flex, true)

	if err := app.Run(); err != nil {
		panic(err)
	}
}
@npxcomplete
Copy link
Author

Trying out more of the examples (which have thus far all worked) I found app.EnableMouse which lets me set focus. The inputs all seem to be behaving as I'd hope for the moment :D

@npxcomplete
Copy link
Author

Seems part of the reason that tab'n in the form demo works is that the form construct micro manages it. I suppose this makes sense as a form is a collection and the construction order defines the tab order then?

https://github.com/rivo/tview/blob/master/form.go#L706-L719

So I wonder then, can I embed the elements of the form in a separate layout, excluding the form entity from widget tree and still benefit from the form's logic?

@npxcomplete
Copy link
Author

Indeed, the use of form.AddFormItem seems to work a treat. It still leaves me wondering why app.SetFocus seems to be falling short though....

Ah, it's because SetRoot was being called before SetFocus. I suppose it's hard to say "set the focus on this element" if the element is not yet part of the application.

@npxcomplete
Copy link
Author

Alas, form.AddFormItem does not support button types, and there's no way to extricate a button from a form if it's created inside.

*tview.Button does not implement tview.FormItem (missing GetFieldHeight method)

So far I'm really liking the library but I think maybe this is one area where two ideas have become overly coupled (form layout and form control logic). It it's all the same to you I'd settle for patching in an AddFormButton function that behaves like AddFormItem.

@rivo
Copy link
Owner

rivo commented Jul 17, 2024

A Form is kind of a convenience type which gives you some features commonly found in entry forms, e.g. some input elements and a "submit" or "cancel" button at the bottom. This convenience comes with some limitations, too, however, as you noticed.

If you want more flexibility, use Flex or Grid. You were off to a good start. When you add your elements to the Flex component, set the focus parameter to true for the element you want focused initially. All of these flags were set to false in your code so no interactive element was focused, that's why nothing happened.

If "focus" is set to true, the item will receive focus when the Flex primitive receives focus.

If you want to navigate between elements with Tab or Backtab, use SetInputCapture() on your Flex and listen for those events. You can then call app.SetFocus() for your elements in the order you desire.

@npxcomplete
Copy link
Author

npxcomplete commented Jul 18, 2024

Indeed, a simple enough function:

func FocusControlGroup(
	app *tview.Application,
	group []tview.Primitive,
) func(event *tcell.EventKey) *tcell.EventKey {
	return func(event *tcell.EventKey) *tcell.EventKey {
		if event.Key() == tcell.KeyTab {
			for i := 0; i < len(group); i++ {
				if group[i].HasFocus() {
					next := (i + 1) % len(group)
					app.SetFocus(group[next])
					return nil
				}
			}
			return nil
		} else if event.Key() == tcell.KeyBacktab {
			for i := 0; i < len(group); i++ {
				if group[i].HasFocus() {
					next := (len(group) + i - 1) % len(group)
					app.SetFocus(group[next])
					return nil
				}
			}
			return nil
		}
		return event
	}
}

Invoked with the controls and provided to the SeInputCapture function yield the desired behavior :D

	flex.SetInputCapture(controls.FocusControlGroup(app, []tview.Primitive{
		logGroupSelect,
		startTime,
		endTime,
		textArea,
		getLogs,
	}))

There's plenty of work in the browser plugin space to define good keyboard navigation, as well as the more application specific implementations like vim and gmail. I can imagine slightly more complex apps using a modal system, something like press T to focus this text box, edit the contents, then press Esc or Enter to blur. I wonder how general or complicated a component can be and still be a welcome addition to your library? Is it worth trying to productionize the above and put up a pull request? If so, I can foresee making some other complicated but generalizable components for this little toy app I'm building:

  • Calendar modal
  • Tab view (as in browser tabs)
  • Line Graph
  • Autocomplete textbox + dropdown

@npxcomplete
Copy link
Author

npxcomplete commented Jul 21, 2024

Apologies if you want this ticket closed. While it's still here I'm tracking it as a bit of a dev diary as I hit other sticking points. Today's sticking point is a virtual crash caused by (what seems to be) a combination of input buffering and excessive drawing of the virtual table example.

Given a virtual table of ~100 elements the up/down arrows correctly scroll the table until the limit. However, it seems that whatever internal counter is being used in the table construct is more than happy to allow the focus index to drift out of bound (i.e. into the negative when scrolling up). Combine this with the fact that Draw is slow, happens for every key input, and that input is buffered and what you get is a user that presses up to scroll to the first list, but when the first row becomes visible there are still thousands, maybe millions of inputs waiting to be processed. The input queue will then take minutes or longer to resolve the queue as Draw eats 100% of the CPU.

There's a knock on effect too. While that input buffer is being resolved ctrl+c isn't registered. So naturally I try to kill the process. Here's where it gets fun, If you kill a tview process it doesn't hand input control back to the terminal properly, so the terminal sees tcell.Key codes printed out but no return to the shell prompt :D

I suppose there are a lot of remediations that could be attempted here.

  • A hard (but large) bound on the input buffer isn't a horrible idea.
  • A soft bound on the number of repeated inputs that can be buffered sequentially.
  • A bound on the virtual table index that negates update once hit.
  • Draw skipping (i.e. process queued inputs in bulk, then call draw once).

I haven't dug in yet, so I'm assuming a bit about how much control the library has over input queuing. That last one presents an interesting option though. Something like a rate limiter on the draw function so any invocations of Draw are dropped after a limit is hit and then a full screen redraw is triggered and burst capacity restored.

@npxcomplete
Copy link
Author

Found the redraw pause (set to 50ms) and the event queue bounded to 100.
https://github.com/rivo/tview/blob/master/application.go#L11

Key-down repetition seems to work out at 32/sec or ~ 1 input per 32ms. Assuming input events are dropped if that channel is full then it would make sense to assume the limit is hit after ~8 seconds. And following key release it would take 5 seconds for the event buffer to clear.

Testing: 10 seconds of up-arrow took ~30sec to resolve following key-up. Suggesting the Draw function is taking ~128ms with no work to do.

Possibly my confusion. The event queue seems to be accepting only QueueUpdateDraw style events, not device input. Negative, they're coming in from tcell.Screen, tcell's terminal screen uses a buffered channel with size 10

https://github.com/rivo/tview/blob/master/application.go#L299
https://github.com/gdamore/tcell/blob/main/tscreen.go#L232

No events are dropped at this level. Looking around, tcell is pulling buffered bytes from the terminal and converting to runes.

https://github.com/gdamore/tcell/blob/main/tscreen.go#L1712

Seems there is room here for selective input dropping. But maybe skipping needless redraws would be the more valuable tactic. On second review the redraw pause behavior only applies to screen resize. So all events queue up, and every keyboard event triggers a redraw.

@rivo
Copy link
Owner

rivo commented Jul 28, 2024

The limits you linked to are for the buffered channels. Once the limit is reached, adding new channel elements will stall. No input gets dropped. If you want input to be dropped, use Application.SetInputCapture().

But I would suggest rather focusing on improving performance of your application. Virtual tables can be quite efficient. The demo exhausts the entire Int64 space, for example:

https://github.com/rivo/tview/blob/master/demos/table/virtualtable/main.go

There have been some comments previously in the issues section regarding decoupling input from drawing. If you really need it, it can be done.

Autocomplete textbox + dropdown

This already exists, by the way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants