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

TUI assumes black background #108

Closed
0-wiz-0 opened this issue Aug 19, 2024 · 14 comments
Closed

TUI assumes black background #108

0-wiz-0 opened this issue Aug 19, 2024 · 14 comments
Labels
bug Something isn't working

Comments

@0-wiz-0
Copy link

0-wiz-0 commented Aug 19, 2024

I've tried HexPatch 1.8.0 on an iTerm2 via ssh to a NetBSD server.
I prefer light mode (i.e. black text on white blackground).
When I start hexpatch and look at the help, I can read basically nothing.

It's fine to only prefer dark mode and default to it, but please make sure that the background really is black.

(It is possible that this works elsewhere and is only a problem because NetBSD because NetBSD has its own curses implementation, not ncurses; but most software works fine.)

(I found https://github.com/Etto48/HexPatch/blob/master/docs/SETTINGS.md in the meantime, but I think the default settings should be usable without configuration.)
Screenshot 2024-08-19 at 15 07 09

@0-wiz-0 0-wiz-0 added the bug Something isn't working label Aug 19, 2024
@0-wiz-0
Copy link
Author

0-wiz-0 commented Aug 19, 2024

Ok, same problem on macOS directly (in iTerm2), so NetBSD's not at fault :)
Screenshot 2024-08-19 at 16 50 01

@0323pin
Copy link
Contributor

0323pin commented Aug 20, 2024

Packaged and merged into pkgsrc now, https://mail-index.netbsd.org/pkgsrc-changes/2024/08/20/msg305907.html

@Etto48
Copy link
Owner

Etto48 commented Aug 25, 2024

Thank you @0-wiz-0 ! I would have never noticed, I will try to fix it asap. If you have already made a light theme, could you send it to me, it would save me some time.

@Etto48
Copy link
Owner

Etto48 commented Aug 25, 2024

Thank you @0323pin ! (For what I understand the message is unrelated to this issue right?) Do you think it's appropriate to add the new installation method for NetBSD in the documentation?

@0-wiz-0
Copy link
Author

0-wiz-0 commented Aug 25, 2024

Thanks for looking at this, @Etto48 ! No, I haven't written a light theme yet.

@Etto48
Copy link
Owner

Etto48 commented Aug 25, 2024

@0-wiz-0 no problem, I'm putting copilot at work, once I find a bit of time to test it I'll push a new release (if it works)

@0323pin
Copy link
Contributor

0323pin commented Aug 25, 2024

Thank you @0323pin ! (For what I understand the message is unrelated to this issue right?) Do you think it's appropriate to add the new installation method for NetBSD in the documentation?

Yes, it's unrelated. It made sense to me, as I've packaged it and asked @0-wiz-0 to test the package before merging 😉

I can do a PR in the coming days to add installation notes.

@Etto48
Copy link
Owner

Etto48 commented Aug 25, 2024

@0-wiz-0 unfortunately I'm not finding a functioning way to detect the background of the terminal, I think that the possible solution is one of:

  • Changing the default theme to be visible both with black and white backgrounds (this might be challenging)
  • Adding a setting to switch between dark and light theme
  • Setting a black background with crossterm and handling everything as if it was black (it's not really a solution as it does not offer a white version + it's quite ugly with transparent backgrounds)
  • Anything you suggest 😅

(I tried to use the crate termbg but it reports that my alacritty installation on windows (which I have set to have a white bg) as "Dark")

@0-wiz-0
Copy link
Author

0-wiz-0 commented Aug 25, 2024

I think the best for starters is the third, to set a black background. Then at least for now it will be usable in most cases. Having light theme support would be nice, but that's more effort and can be second step.

The problem with termbg sounds like a bug that needs reporting upstream :)

@Etto48
Copy link
Owner

Etto48 commented Aug 25, 2024

Thank you @0323pin ! (For what I understand the message is unrelated to this issue right?) Do you think it's appropriate to add the new installation method for NetBSD in the documentation?

Yes, it's unrelated. It made sense to me, as I've packaged it and asked @0-wiz-0 to test the package before merging 😉

I can do a PR in the coming days to add installation notes.

Ahh now I understand, I'd appreciate that <3

@Etto48
Copy link
Owner

Etto48 commented Aug 25, 2024

I think the best for starters is the third, to set a black background. Then at least for now it will be usable in most cases. Having light theme support would be nice, but that's more effort and can be second step.

The problem with termbg sounds like a bug that needs reporting upstream :)

I'll try termbg on other platforms before resorting to that option, because changing the default bg to anything other than "Reset" breaks the default theme.

@Etto48
Copy link
Owner

Etto48 commented Aug 25, 2024

@0-wiz-0 Ok, just tested on a more common environment and it's working, I will use termbg to detect the background and maybe create an issue on it.

@Etto48
Copy link
Owner

Etto48 commented Aug 25, 2024

I created an issue on termbg (dalance/termbg#25) where I explain the problem, for the time being I will use a copied and fixed version of the library as it's really small.

@Etto48
Copy link
Owner

Etto48 commented Aug 29, 2024

So, I'm closing the issue with this merge because the light theme seems to be working, unfortunately on windows there is minimal support for automatic theme detection, I will wait a bit before making a new release to see if dalance can fix the library.

@Etto48 Etto48 closed this as completed Aug 29, 2024
@Etto48 Etto48 mentioned this issue Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants