Etto48 / HexPatch

HexPatch: a binary patcher and editor written in Rust with terminal user interface (TUI).
https://etto48.github.io/HexPatch/
MIT License
158 stars 7 forks source link

TUI assumes black background #108

Closed 0-wiz-0 closed 2 months ago

0-wiz-0 commented 2 months ago

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 commented 2 months ago

Ok, same problem on macOS directly (in iTerm2), so NetBSD's not at fault :)

Screenshot 2024-08-19 at 16 50 01
0323pin commented 2 months ago

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

Etto48 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

Etto48 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

@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:

(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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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.