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

pty fallback term #511

Merged
merged 3 commits into from
Jul 13, 2023
Merged

pty fallback term #511

merged 3 commits into from
Jul 13, 2023

Conversation

luis-pereira
Copy link
Member

  • Prefer ranged for over while
  • Add a TERM fallback to the pseudoterminal (PTY)

Simpler and more readable.
The pseudoterminal should always run with the TERM environment variable set.
xterm-256color is a sensible fallback.
@yan12125
Copy link
Member

Looks like this fixed #447 ?

@luis-pereira
Copy link
Member Author

Yes, it fixes it as "secondary" effect.
Default TERM from qterminal needs to be set in qtermwidget. Will check how to do that.

@yan12125
Copy link
Member

qterminal sets TERM via qputenv. Other applications may do something similar. Looks like this approach overwrites existing environment variables?

@luis-pereira
Copy link
Member Author

qterminal sets TERM via qputenv. Other applications may do something similar. Looks like this approach overwrites existing environment variables?

@yan12125 Is there any downside to lxqt/qterminal#1058 ?

@luis-pereira
Copy link
Member Author

@yan12125 xterm is a even more sensible fallback.

@yan12125
Copy link
Member

Is there any downside to lxqt/qterminal#1058 ?

For qterminal, I don't think so. There might still be an issue for other applications that embed qterm widget. Could you add some sentences to https://github.com/lxqt/qtermwidget/blob/master/CHANGELOG to clarify that?

By the way, lxqt/qterminal#406 may be fixed as a side effect.

xterm is a even more sensible fallback.

I prefer xterm-256color. qtermwidget already supports most of commonly-used features in xterm-256color.

@luis-pereira
Copy link
Member Author

For qterminal, I don't think so. There might still be an issue for other applications that embed qterm widget. Could you add some sentences to https://github.com/lxqt/qtermwidget/blob/master/CHANGELOG to clarify that?

Ok. Will do that.
What issue for apps that embed qtermwidget ? If they don't set TERM, qtermwidget set it to xterm-256color as a fallback. A method to explicilty do that can be added.

By the way, lxqt/qterminal#406 may be fixed as a side effect.

Good. I wasn't even aware of lxqt/qterminal#406.

I prefer xterm-256color. qtermwidget already supports most of commonly-used features in xterm-256color.

Agrreed.

@yan12125
Copy link
Member

yan12125 commented Jul 1, 2023

If an application sets TERM by qputenv (like the current qterminal), will it be overwritten? If so, a migration hint is good.

Note about the TERM variable.
@yan12125 yan12125 merged commit c6fa21e into master Jul 13, 2023
2 checks passed
@yan12125 yan12125 deleted the pty-fallback-term branch July 13, 2023 05:33
@yan12125
Copy link
Member

Thanks! I use rebase instead of squash as the first commit is a different improvement.

This pull request was closed.
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.

2 participants