Skip to content

Backport Windows support from KDE Konsole #578

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

loliting
Copy link

@loliting loliting commented Jan 1, 2025

Backport Windows support from KDE Konsole

unistd.h is not avaliable on MSVC, but we do not use any functions from it
BlockArray based history types are now disabled when Q_OS_WIN is defined.
Also other history types do not use mmap to map files now.
Backported from konsole
was added to UNIX_SRCS in one of the previous commits and i forgot to remove it from SRCS
This commit also introduces 2 new public helper functions for Session::close():
closeInNormalWay() and closeInForceWay(). These are backported from KDE Konsole
Courier New is only default monospaced font on windows
@tsujan
Copy link
Member

tsujan commented Jan 1, 2025

I really doubt that this can be included in LXQt. Why don't you make a Windows fork instead?

@loliting
Copy link
Author

loliting commented Jan 1, 2025

Why though? This doesn’t really break any existing code and support for windows was requested in #42 (and btw it was mentioned there that PRs would be merged), #491 & #494 . ConPTY code is borrowed on the same basics as KPty stuff (and is MIT licensed).

I really don’t want to fork whole library, because then I would have to maintain entirety of it.

@tsujan
Copy link
Member

tsujan commented Jan 1, 2025

Because LXQt doesn't have the required manpower to support Windows.

… then I would have to maintain entirety of it.

Exactly ;) That's what LXQt can't do either.

@loliting
Copy link
Author

loliting commented Jan 1, 2025

Okay, that makes sense.

What if I could help with maintaining windows support?

@tsujan
Copy link
Member

tsujan commented Jan 1, 2025

That would be very nice.

But the question remains as to who will review this PR? As the current maintainer of qterminal+qtermwidget, I don't (and won't) have Windows (to say nothing of the time).

So, I suggest that we could wait to see if an LXQt dev will review it, knowing that there's no promise.

And please don't get me wrong: your contribution is very appreciated — I know how hard it may have been to do this — but this area may be outside LXQt. I'm just talking about facts.

@tsujan
Copy link
Member

tsujan commented Jan 1, 2025

BTW, all the reports you mentioned above were closed. I refer you to the last reply of the previous maintainer: #491 (comment)

@loliting
Copy link
Author

loliting commented Jan 1, 2025

So, I suggest that we could wait to see if an LXQt dev will review it, knowing that there's no promise.

Okay then.

I needed to implement windows support for one of my personal projects (which depends on qtermwidget), so I ported Windows support here. I don't have a need for it to be upstreamed, but I thought this PR could be of use, so there isn't any pressure from my part.

@doug1234
Copy link
Contributor

doug1234 commented Jun 4, 2025

I know I am late to the discussion but this is something I am interested in @loliting . Any chance you would be interested in creating a fork that has windows support?

@KangLin
Copy link

KangLin commented Jun 17, 2025

That would be very nice.

But the question remains as to who will review this PR? As the current maintainer of qterminal+qtermwidget, I don't (and won't) have Windows (to say nothing of the time).

@tsujan If I help you with the review, is there an opportunity to merge it?

@KangLin
Copy link

KangLin commented Jun 17, 2025

@loliting It's a great job. It is recommended to add CI

@KangLin
Copy link

KangLin commented Jun 17, 2025

Try memoryapi to replace mmap

KangLin
KangLin approved these changes Jun 17, 2025
@tsujan
Copy link
Member

tsujan commented Jun 17, 2025

@tsujan If I help you with the review, is there an opportunity to merge it?

No. Please read above.

@KangLin
Copy link

KangLin commented Jun 18, 2025

@tsujan If I help you with the review, is there an opportunity to merge it?

No. Please read above.

Still want to support Windows, it's not expensive to maintain. There are many people who can help you.

@KangLin
Copy link

KangLin commented Jun 26, 2025

@loliting The startTerminalTeletype is not work?

@KangLin
Copy link

KangLin commented Jul 2, 2025

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

Successfully merging this pull request may close these issues.

4 participants