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

Lipgloss doesn't detect background correctly when using wish #262

Closed
Aerochrome opened this issue Apr 26, 2024 · 10 comments
Closed

Lipgloss doesn't detect background correctly when using wish #262

Aerochrome opened this issue Apr 26, 2024 · 10 comments

Comments

@Aerochrome
Copy link

Describe the bug
When using the bubbletea.MakeRenderer function, lipgloss.AdaptiveColor and lipgloss.Renderer.HasDarkBackground always assume a black background, even though the terminal emulator from which i am using ssh has set a light background.

Setup
I'm running the wish example locally with go run . and also use a local terminal to access the ssh server.
It happens on Fedora 40 and MacOS with Gnome Terminal, iTerm2 and the standard mac terminal (all using zsh).

To Reproduce

  1. Use the bubbletea example from the examples folder
  2. Log HasDarkBackground using
    log.Info(renderer.HasDarkBackground()) in the teaHandler function right after creating the renderer using the bubbletea middleware
  3. Create a new Lipgloss Style in the teaHandler function that uses the renderer created using the bubbletea middleware and lipgloss.AdaptiveColor
    3.1 Render some text in the View() method of the model using the adaptive color lipgloss style

Source Code
I've created a simple one-file example in this gist:
https://gist.github.com/Aerochrome/5fd2af09f94eda22a0875068b8824acc

Expected behavior
When accessing with a terminal emulator with a light background, the second line should be colored pink.
When accessing with a dark background, the second line should be colored red.

Screenshots
Expectation:
gist_working

Reality:
gist_not_working

Additional context
DISCLAIMER: I am very new to TUIs and fairly new to go, so i'm not sure how useful my debugging findings actually are, but i want to share them nonetheless in case they're of any use.

I suspect that this issue might be related to a missing Fd() method implementation on the ssh.Session struct.
In the bubbletea.newRenderer method in this file a check is performed on whether the Slave pointer is nil. If so, it passes the session struct into lipgloss.NewRenderer.

While this struct has a Write method, it doesn't seem to implement the Fd method.

When adding a very rudimentary one like so, the background color check seems to work properly and produces a working example.

func (sess *session) Fd() uintptr {
    return sess.pty.Slave.Fd() // Also works if sess.pty.Slave is nil
}

Of course, this doesn't account for sess.pty.Slave being nil (even though it produces a working example even if it is nil and i'm unsure why).

This suspicion is further confirmed by looking at an older wish example in the lipgloss repository, where a "helper" struct is created to pass into lipgloss.Renderer.SetOutput which also implements Write and Fd.
It also implements Read, though it doesn't seem to influence the outcome of a working example in my case.

So i've tried to use this helper struct, setting tty to pty.Slave and passing it into bubbletea.MakeRenderer like so, which produces a working example.

sessionBridge := &sshOutput{
    Session: s,
    tty: pty.Slave,
}

renderer := bubbletea.MakeRenderer(sessionBridge)

Again, this also works if pty.Slave is nil.

It is also worth mentioning that in my case checking pty.Master and pty.Slave against nil in the teaHandler function returns true in both cases, even though s.Pty() returns true as third return value which i assume means that a PTY was accepted for this session.
I don't know if this is expected behavior in this specific case?

log.Info(pty.Master == nil)
log.Info(pty.Slave == nil)
@adamdottv
Copy link

Also running into this! Happy to help if there's any opportunity to do so.

caarlos0 added a commit to muesli/termenv that referenced this issue May 2, 2024
A pty is not in foreground, but it can do the IOCTL ops we need.

My understanding might be wrong, but I think that if the term cannot do
the TCGETS it will fail the same way it would when checking if its in
foreground, so maybe that check is even needed?

refs charmbracelet/wish#262
@caarlos0
Copy link
Member

caarlos0 commented May 2, 2024

FWIW we're looking into it :)

thanks for the detailed report

@Aerochrome
Copy link
Author

Awesome, thank you!
Glad i was able to help :)

caarlos0 added a commit that referenced this issue May 2, 2024
use our new term lib to query the bg, set it into the lipgloss renderer
so it works with everything.

refs #262
caarlos0 added a commit that referenced this issue May 2, 2024
use our new term lib to query the bg, set it into the lipgloss renderer
so it works with everything.

refs #262
@caarlos0
Copy link
Member

caarlos0 commented May 2, 2024

can you try on main?

@Aerochrome
Copy link
Author

Sure!

I've tried to run the example i provided in the gist with the main branch of wish, however the issue still persists.

I've executed

  • go get github.com/charmbracelet/wish@main
  • go mod tidy

And i am now on version v1.4.1-0.20240502183613-927ad4fbed5f of wish.

Perhaps there is something else i'm missing?

@caarlos0
Copy link
Member

caarlos0 commented May 2, 2024

afaik that should be it, can you share the project you're working on?

@Aerochrome
Copy link
Author

Of course!

I've pushed my test project here: https://github.com/Aerochrome/charm-bg-test

@caarlos0
Copy link
Member

caarlos0 commented May 3, 2024

right now it'll only work if you use allocatepty, I'm afraid

still looking into it though

@Aerochrome
Copy link
Author

Aerochrome commented May 3, 2024

I can confirm that on wish v1.4.1-0.20240503145853-34bb46304c87 it now works as expected without allocatepty 🎉

Thanks!

@caarlos0 caarlos0 closed this as completed May 3, 2024
@caarlos0
Copy link
Member

caarlos0 commented May 3, 2024

awesome, thank you!

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

3 participants