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

Support brick-2 and vty-6. #2112

Closed
amano-kenji opened this issue Nov 16, 2023 · 15 comments
Closed

Support brick-2 and vty-6. #2112

amano-kenji opened this issue Nov 16, 2023 · 15 comments
Labels
A-WISH Some kind of improvement request, hare-brained proposal, or plea. packaging Dependencies, version constraints, packaging.. ui The hledger-ui tool.

Comments

@amano-kenji
Copy link

brick-2 depends on vty-6 which introduces breaking changes.

From vty-6 onward, you have to depend on vty-crossplatform, vty-windows, or vty-unix for mkVty function.

If you don't use mkVty function, then you don't need to depend on one of platform-specific vty packages.

@amano-kenji amano-kenji added the A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. label Nov 16, 2023
@simonmichael simonmichael added A-WISH Some kind of improvement request, hare-brained proposal, or plea. packaging Dependencies, version constraints, packaging.. ui The hledger-ui tool. and removed A-BUG Something wrong, confusing or sub-standard in the software, docs, or user experience. labels Nov 16, 2023
@ShrykeWindgrace
Copy link

ShrykeWindgrace commented Nov 22, 2023

I am working on this. In theory, this requires very minimal change in hledger-ui:

  • fix some imports (a couple of lines)
  • fix build conditions (delete a couple of lines)
  • add lower bounds for brick(/vty/vty-crossplatform, because stack snapshots do not have new versions yet (also a very local change).
  • add notepad as default editor on windows; git for windows does that, for example (diff is a couple of lines)
  • change CI scripts to release windows executable for hledger-ui. I haven't looked into that yet; I'll need some help with that

There is, however, a bug:

  • run hledger-ui
  • exit hledger-ui (q)
  • terminal no longer responds to mouse ⚠️ !

I'll see whether the bug is in hledger-ui, brick, or vty-windows (or in my setup 😢 ). I am not comfortable with pushing for windows availability of hledger-ui before fixing this bug.

@simonmichael
Copy link
Owner

Wonderful ! I was thinking about this, thanks for working on it.

@simonmichael
Copy link
Owner

(It will probably not get merged in master until stackage nightly gets brick 2.)

@simonmichael
Copy link
Owner

I'm happy to handle the CI part.

@simonmichael
Copy link
Owner

(Sorry for the stream of messages, last one:)

It's worth testing in as many windows terminal environments as you can find, we'll probably get different bug reports for each one.

@amano-kenji
Copy link
Author

Did you try latest versions of vty and brick? The latest versions might have fixed the rendering issue.

@ShrykeWindgrace
Copy link

(Sorry for the stream of messages, last one:)

It's worth testing in as many windows terminal environments as you can find, we'll probably get different bug reports for each one.

I guess most of this bugreports will be safely redirected to vty/vty-windows in that case, with all being "ok, in that terminal emulator it almost works, except for that and that feature" =) We can explicitly mention that hledger-ui will be ok for Windows Terminal, ok-ish in ListOfTerminalEmulators, and most probably not ok in AnotherListOfTerminalEmulators.

@ShrykeWindgrace
Copy link

Did you try latest versions of vty and brick? The latest versions might have fixed the rendering issue.

Yes, I tested both master and latest releases. This is not a rendering issue, it is a "restore terminal to its prior state" issue.

@ShrykeWindgrace
Copy link

Well, for now the list of supported terminal emulators is a WIP, https://github.com/chhackett/vty-windows/wiki/TerminalSupport with latest info. The bug with terminal state is fixed, I'll open the PR in the next couple of days.

Repository owner deleted a comment from ShrykeWindgrace Dec 7, 2023
Repository owner deleted a comment from the-solipsist Dec 7, 2023
Repository owner deleted a comment from ShrykeWindgrace Dec 7, 2023
Repository owner deleted a comment from the-solipsist Dec 7, 2023
@simonmichael
Copy link
Owner

simonmichael commented Dec 7, 2023

could you move the thread on kitty to a separate issue?

Done, #2126.

Related to this issue, hledger-ui is now out of stackage nightly. I'm assuming supporting both old and new vty in the same release would be too much CPP and hassle (?), so getting back in to stackage will require a new release that supports and requires vty 6.1+ and brick 2.1.1+. Our 1.32.1 bugfix release is pending. But probably squeezing a big vty/brick upgrade into a bugfix release would be a bad idea.

@simonmichael
Copy link
Owner

I might do a separate hledger-ui-1.32.1 release after this has landed and we have had some cross platform user testing. (Need to figure out simpler release process for individual packages..)

@simonmichael
Copy link
Owner

Though, windows support is a feature worth shouting about, not a bugfix..

@ShrykeWindgrace
Copy link

ShrykeWindgrace commented Dec 8, 2023

Done, #2126.

Thank you!

On-topic: since we got vty-6, vty-crossplatform and brick-2 stackage, I'll prepare a PR with my changes.

Though, windows support is a feature worth shouting about, not a bugfix..

Indeed =)

@amano-kenji
Copy link
Author

amano-kenji commented Jan 8, 2024

I'm waiting for brick 2.3 support because brick 2.3 has functions that I want.

@amano-kenji
Copy link
Author

I think it can be closed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-WISH Some kind of improvement request, hare-brained proposal, or plea. packaging Dependencies, version constraints, packaging.. ui The hledger-ui tool.
Projects
None yet
Development

No branches or pull requests

3 participants