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

ncurses terminal resize improvement #1235

Closed
jubalh opened this issue Dec 1, 2019 · 44 comments
Closed

ncurses terminal resize improvement #1235

jubalh opened this issue Dec 1, 2019 · 44 comments
Labels
Milestone

Comments

@jubalh
Copy link
Member

jubalh commented Dec 1, 2019

I think somewhere ncurses redraw/refresh/update functions are not used properly.

If we change the terminals window size profanity is not redrawn correctly. Also some parts of the screen are not updated correctly but I don't know how to reproduce it yet and might create another issue later.

In this example I run profanity in gnome-terminal.

  1. start profanity:
    prof-1

  2. Press F11 to have fullscreen:
    prof-2

Already now it looks weird.

  1. Press F11 again to have the regular window size again:
    prof-3

Looks even weirder.

@jubalh jubalh added this to the 0.8.0 milestone Dec 1, 2019
@jubalh
Copy link
Member Author

jubalh commented Dec 1, 2019

@xaizek sorry to bother you with this. But I know you know ncurses well. Do you have an idea what is happening here?

@jubalh jubalh changed the title ncurses improvement ncurses terminal resize improvement Dec 1, 2019
@mdosch
Copy link
Contributor

mdosch commented Dec 1, 2019

I don't have this problem with gnome-terminal on Debian Testing with i3/X.org:

#  GNOME Terminal 3.34.2 using VTE 0.58.2 +BIDI +GNUTLS

It works with F11 as well as with Super+F (i3 fullscreen).

@xaizek
Copy link

xaizek commented Dec 1, 2019

In functions like status_bar_resize() and title_bar_resize() windows are moved before being resized. The operations should be done in the opposite order (resize and then move), because

If the move would cause the window to be off the screen, it is an error and the window
is not moved.

Resize on the other hand doesn't fail like this according to its documentation. So new size needs to be applied first.

Not sure if it will make all issues go away, but some might be fixed. I just glanced over the code and didn't actually build and run it. Fix order of operations and will see if anything else needs to be done.

@jubalh
Copy link
Member Author

jubalh commented Dec 2, 2019

@xaizek thanks for the feedback!

So my local code looks like this now:

diff --git a/src/ui/statusbar.c b/src/ui/statusbar.c
index 39b1ddbb..8f9900e1 100644
--- a/src/ui/statusbar.c
+++ b/src/ui/statusbar.c
@@ -137,8 +137,8 @@ status_bar_resize(void)
     int cols = getmaxx(stdscr);
     werase(statusbar_win);
     int row = screen_statusbar_row();
-    mvwin(statusbar_win, row, 0);
     wresize(statusbar_win, 1, cols);
+    mvwin(statusbar_win, row, 0);
 
     status_bar_draw();
 }
diff --git a/src/ui/titlebar.c b/src/ui/titlebar.c
index 9cb2cfec..50b85932 100644
--- a/src/ui/titlebar.c
+++ b/src/ui/titlebar.c
@@ -108,9 +108,10 @@ title_bar_resize(void)
     werase(win);
 
     int row = screen_titlebar_row();
-    mvwin(win, row, 0);
 
     wresize(win, 1, cols);
+    mvwin(win, row, 0);
+
     wbkgd(win, theme_attrs(THEME_TITLE_TEXT));
 
     _title_bar_draw();

Unfortunately it still behaves the same for me though.

Any more ideas?

@jubalh
Copy link
Member Author

jubalh commented Dec 2, 2019

The interesting part is: I do resize several times, and sometimes it works and sometimes it doesn't. Mostly it doesn't.

@xaizek
Copy link

xaizek commented Dec 2, 2019

I also can't reproduce it. And statusline looks different here. Probably because I have no settings. How to configure it to look like yours? The information displayed there might affect widths of things.

@jubalh
Copy link
Member Author

jubalh commented Dec 2, 2019

I also can't reproduce it. And statusline looks different here. Probably because I have no settings.

That's weird. I just created a new user on my system. So I don't have any configuration files. And did the exact same thing. And it still occurs here.

My theme for this new user (can be seen with /theme) is: default.

So this should be the same as yours.

The screenshot I posted is from another PC, I will try to post the settings when I'm back home.

However it is weird that for me it still happens with a new user with no settings.

@jubalh
Copy link
Member Author

jubalh commented Dec 2, 2019

I just tried with qterminal and xterm. The problem doesn't occur there for me.
My gnome-terminal version is 3.34.2.

jubalh added a commit that referenced this issue Dec 2, 2019
From @xaizek s comment on issue #1235:
```
If the move would cause the window to be off the screen, it is an error and the window
is not moved.

Resize on the other hand doesn't fail like this according to its documentation. So new size needs to be applied first.
```

Big thanks to @xaizek for taking a look at our code and helping us!!

Regards #1235
@xaizek
Copy link

xaizek commented Dec 2, 2019

for me it still happens with a new user with no settings.

Then settings probably don't matter.

The problem doesn't occur there for me.

I tried it with xterm, screen inside xterm, urxvt, st and vte (sample terminal for the library). Maybe it is a bug in libvte. You could try one of the other libvte-based terminals on the same system where you try gnome-terminal:

  • sakura
  • xfce4-terminal
  • Terminator
  • Tilix
  • Lilyterm
  • ROXTerm
  • evilvte
  • Termit
  • Termite
  • Tilda
  • tinyterm
  • Pantheon Terminal
  • lxterminal
  • guake

@jubalh
Copy link
Member Author

jubalh commented Dec 3, 2019

I tried it with xfce4-terminal and terminator. In both cases the problem occurs!
Interesting..

I have libvte 0.58.2. @mdosch which version do you have?

It seems there is already 0.58.3 out. It's changelog sais:

    - Report fixed origin for CSI 13 t.
    - Maintain cursor column during screen switch.
    - Fix vertical cursor movememnt outside the scroll region.

Will update to this.

@jubalh
Copy link
Member Author

jubalh commented Dec 11, 2019

Even with the latest update this happens for me.

I also took another computer and did a fresh install of openSUSE Tumbleweed there. It also happens there for me.

Will test on another distro. And will check the patches for libvte applied by openSUSE.

@jubalh
Copy link
Member Author

jubalh commented Dec 11, 2019

Seems no patches in libvte nor gnome-terminal in openSUSE.

@mdosch
Copy link
Contributor

mdosch commented Dec 11, 2019 via email

@jubalh
Copy link
Member Author

jubalh commented Dec 12, 2019

Another user confirmed that it works on Debian Testing with xfce4-terminal 0.8.8.

@jubalh
Copy link
Member Author

jubalh commented Dec 12, 2019

I'm running ncurses 6.1 with a few patches.

@mdosch how about you?

@jubalh
Copy link
Member Author

jubalh commented Dec 12, 2019

Heard from another user that it works fine on Arch Linux with: profanity 0.7.1, ncurses 6.1, Terminator

@jubalh
Copy link
Member Author

jubalh commented Dec 12, 2019

/etc/profile.d/vte.sh on both my openSUSE Tumbleweed and the Arch machine look like this:

# Copyright © 2006 Shaun McCance <shaunm@gnome.org>
# Copyright © 2013 Peter De Wachter <pdewacht@gmail.com>
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, see <http://www.gnu.org/licenses/>.

# Not bash or zsh?
[ -n "$BASH_VERSION" -o -n "$ZSH_VERSION" ] || return 0

# Not an interactive shell?
[[ $- == *i* ]] || return 0

# Not running under vte?
[ "${VTE_VERSION:-0}" -ge 3405 ] || return 0

__vte_urlencode() (
  # This is important to make sure string manipulation is handled
  # byte-by-byte.
  LC_ALL=C
  str="$1"
  while [ -n "$str" ]; do
    safe="${str%%[!a-zA-Z0-9/:_\.\-\!\'\(\)~]*}"
    printf "%s" "$safe"
    str="${str#"$safe"}"
    if [ -n "$str" ]; then
      printf "%%%02X" "'$str"
      str="${str#?}"
    fi
  done
)

# Print a warning so that anyone who's added this manually to his PS1 can adapt.
# The function will be removed in a later version.
__vte_ps1() {
  echo -n "(__vte_ps1 is obsolete)"
}

__vte_osc7 () {
  printf "\033]7;file://%s%s\033\\" "${HOSTNAME:-}" "$(__vte_urlencode "${PWD}")"
}

__vte_prompt_command() {
  local pwd='~'
  [ "$PWD" != "$HOME" ] && pwd=${PWD/#$HOME\//\~\/}
  pwd="${pwd//[[:cntrl:]]}"
  printf "\033]0;%s@%s:%s\033\\%s" "${USER}" "${HOSTNAME%%.*}" "${pwd}" "$(__vte_osc7)"
}

case "$TERM" in
  xterm*|vte*)
    [ -n "$BASH_VERSION" ] && PROMPT_COMMAND="__vte_prompt_command"
    [ -n "$ZSH_VERSION"  ] && precmd_functions+=(__vte_osc7)
    ;;
esac

true

@mdosch
Copy link
Contributor

mdosch commented Dec 12, 2019

ncurses-bin/testing,unstable,now 6.1+20191019-1

zsh/testing,unstable,now 5.7.1-1+b1

cat /etc/profile.d/vte-2.91.sh      
# Copyright © 2006 Shaun McCance <shaunm@gnome.org>
# Copyright © 2013 Peter De Wachter <pdewacht@gmail.com>
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program.  If not, see <http://www.gnu.org/licenses/>.

# Not bash or zsh?
[ -n "$BASH_VERSION" -o -n "$ZSH_VERSION" ] || return 0

# Not an interactive shell?
[[ $- == *i* ]] || return 0

# Not running under vte?
[ "${VTE_VERSION:-0}" -ge 3405 ] || return 0

__vte_urlencode() (
  # This is important to make sure string manipulation is handled
  # byte-by-byte.
  LC_ALL=C
  str="$1"
  while [ -n "$str" ]; do
    safe="${str%%[!a-zA-Z0-9/:_\.\-\!\'\(\)~]*}"
    printf "%s" "$safe"
    str="${str#"$safe"}"
    if [ -n "$str" ]; then
      printf "%%%02X" "'$str"
      str="${str#?}"
    fi
  done
)

# Print a warning so that anyone who's added this manually to his PS1 can adapt.
# The function will be removed in a later version.
__vte_ps1() {
  echo -n "(__vte_ps1 is obsolete)"
}

__vte_osc7 () {
  printf "\033]7;file://%s%s\033\\" "${HOSTNAME:-}" "$(__vte_urlencode "${PWD}")"
}

__vte_prompt_command() {
  local pwd='~'
  [ "$PWD" != "$HOME" ] && pwd=${PWD/#$HOME\//\~\/}
  pwd="${pwd//[[:cntrl:]]}"
  printf "\033]0;%s@%s:%s\033\\%s" "${USER}" "${HOSTNAME%%.*}" "${pwd}" "$(__vte_osc7)"
}

case "$TERM" in
  xterm*|vte*)
    [ -n "$BASH_VERSION" ] && PROMPT_COMMAND="__vte_prompt_command"
    [ -n "$ZSH_VERSION"  ] && precmd_functions+=(__vte_osc7)
    ;;
esac

true

@bitstreamout
Copy link

I'm running ncurses 6.1 with a few patches.

Please try out which values of the TERM variable does work correct, I mean other then "xterm" or "xterm-256color" ... I think about "gnome" and "gnome-256color" for gnome, "xfce " for xfce4-terminal, "terminator" for Terminator, ... and so on. Maybe an older XTerm version might also work like "xterm-old", "xterm-r5", "xterm-r6", "xterm-basic", ...

The list can be found below /usr/share/terminfo/ ...

@jubalh
Copy link
Member Author

jubalh commented Dec 13, 2019

When running gnome-terminal $TERM is set to xterm-256color. xterm is set to xterm. qterminal it is set to xterm-256color too. xfce4-terminal is also xterm-256color.

So it works under qterminal with xterm-256color but not in gnome-terminal and xfce4-terminal.
And it works in xterm with xterm.

After export TERM=gnome-256color in gnome-terminal I get the same faulty behaviour.

If I run tmux in gnome-terminal and (where $TERM is screen-256color) Profanity works fine even in gnome-terminal.

@bitstreamout
Copy link

Does it work with the original ncurses "xterm-new" instead of "xterm" or "xterm-256color" ... nevertheless the gnome variant is based on "vte" terminal type(s) ... there had been a few changes in terminfo.src up to upstream patch level ncurses-6.1-20191207.patch.

@jubalh
Copy link
Member Author

jubalh commented Dec 13, 2019

export TERM=xterm-new in gnome-terminal also does not work.

@bitstreamout
Copy link

Is there any other ncurses based application which shows this problem on current Tumbleweed with gnome-terminal or xfce4-terminal ... like yast with ncurses GUI

@jubalh
Copy link
Member Author

jubalh commented Dec 13, 2019

I just tested yast and newsbeuter but they both work.

So you are thinking its a profanity issue? But why only on oS TW? Also I don't see anything that's wrong in the code, and have been looking for hours.

@bitstreamout
Copy link

It a guess, yes, but only a guess. One could compare the output of infocmp -1 -T xterm written into a file on Tumbleweed and the same on e.g. Debian "Sid", and afterwards running diff -uwp on the two files.

@jubalh
Copy link
Member Author

jubalh commented Dec 13, 2019

on os TW:

#	Reconstructed via infocmp from file: /usr/share/terminfo/x/xterm
xterm|xterm terminal emulator (X Window System),
	am,
	bce,
	km,
	mc5i,
	mir,
	msgr,
	npc,
	xenl,
	colors#8,
	cols#80,
	it#8,
	lines#24,
	pairs#64,
	acsc=``aaffggiijjkkllmmnnooppqqrrssttuuvvwwxxyyzz{{||}}~~,
	bel=^G,
	blink=\E[5m,
	bold=\E[1m,
	cbt=\E[Z,
	civis=\E[?25l,
	clear=\E[H\E[2J,
	cnorm=\E[?12l\E[?25h,
	cr=\r,
	csr=\E[%i%p1%d;%p2%dr,
	cub=\E[%p1%dD,
	cub1=^H,
	cud=\E[%p1%dB,
	cud1=\n,
	cuf=\E[%p1%dC,
	cuf1=\E[C,
	cup=\E[%i%p1%d;%p2%dH,
	cuu=\E[%p1%dA,
	cuu1=\E[A,
	cvvis=\E[?12;25h,
	dch=\E[%p1%dP,
	dch1=\E[P,
	dim=\E[2m,
	dl=\E[%p1%dM,
	dl1=\E[M,
	ech=\E[%p1%dX,
	ed=\E[J,
	el=\E[K,
	el1=\E[1K,
	flash=\E[?5h$<100/>\E[?5l,
	home=\E[H,
	hpa=\E[%i%p1%dG,
	ht=^I,
	hts=\EH,
	ich=\E[%p1%d@,
	il=\E[%p1%dL,
	il1=\E[L,
	ind=\n,
	indn=\E[%p1%dS,
	invis=\E[8m,
	is2=\E[!p\E[?3;4l\E[4l\E>,
	kDC=\E[3;2~,
	kEND=\E[1;2F,
	kHOM=\E[1;2H,
	kIC=\E[2;2~,
	kLFT=\E[1;2D,
	kNXT=\E[6;2~,
	kPRV=\E[5;2~,
	kRIT=\E[1;2C,
	ka1=\EOw,
	ka3=\EOy,
	kb2=\EOu,
	kbs=^?,
	kc1=\EOq,
	kc3=\EOs,
	kcbt=\E[Z,
	kcub1=\EOD,
	kcud1=\EOB,
	kcuf1=\EOC,
	kcuu1=\EOA,
	kdch1=\E[3~,
	kend=\EOF,
	kent=\EOM,
	kf1=\EOP,
	kf10=\E[21~,
	kf11=\E[23~,
	kf12=\E[24~,
	kf13=\E[1;2P,
	kf14=\E[1;2Q,
	kf15=\E[1;2R,
	kf16=\E[1;2S,
	kf17=\E[15;2~,
	kf18=\E[17;2~,
	kf19=\E[18;2~,
	kf2=\EOQ,
	kf20=\E[19;2~,
	kf21=\E[20;2~,
	kf22=\E[21;2~,
	kf23=\E[23;2~,
	kf24=\E[24;2~,
	kf25=\E[1;5P,
	kf26=\E[1;5Q,
	kf27=\E[1;5R,
	kf28=\E[1;5S,
	kf29=\E[15;5~,
	kf3=\EOR,
	kf30=\E[17;5~,
	kf31=\E[18;5~,
	kf32=\E[19;5~,
	kf33=\E[20;5~,
	kf34=\E[21;5~,
	kf35=\E[23;5~,
	kf36=\E[24;5~,
	kf37=\E[1;6P,
	kf38=\E[1;6Q,
	kf39=\E[1;6R,
	kf4=\EOS,
	kf40=\E[1;6S,
	kf41=\E[15;6~,
	kf42=\E[17;6~,
	kf43=\E[18;6~,
	kf44=\E[19;6~,
	kf45=\E[20;6~,
	kf46=\E[21;6~,
	kf47=\E[23;6~,
	kf48=\E[24;6~,
	kf49=\E[1;3P,
	kf5=\E[15~,
	kf50=\E[1;3Q,
	kf51=\E[1;3R,
	kf52=\E[1;3S,
	kf53=\E[15;3~,
	kf54=\E[17;3~,
	kf55=\E[18;3~,
	kf56=\E[19;3~,
	kf57=\E[20;3~,
	kf58=\E[21;3~,
	kf59=\E[23;3~,
	kf6=\E[17~,
	kf60=\E[24;3~,
	kf61=\E[1;4P,
	kf62=\E[1;4Q,
	kf63=\E[1;4R,
	kf7=\E[18~,
	kf8=\E[19~,
	kf9=\E[20~,
	kfnd=\E[1~,
	khome=\EOH,
	kich1=\E[2~,
	kind=\E[1;2B,
	kmous=\E[<,
	knp=\E[6~,
	kpp=\E[5~,
	kri=\E[1;2A,
	kslt=\E[4~,
	mc0=\E[i,
	mc4=\E[4i,
	mc5=\E[5i,
	meml=\El,
	memu=\Em,
	mgc=\E[?69l,
	op=\E[39;49m,
	rc=\E8,
	rep=%p1%c\E[%p2%{1}%-%db,
	rev=\E[7m,
	ri=\EM,
	rin=\E[%p1%dT,
	ritm=\E[23m,
	rmacs=\E(B,
	rmam=\E[?7l,
	rmcup=\E[?1049l\E[23;0;0t,
	rmir=\E[4l,
	rmkx=\E[?1l\E>,
	rmm=\E[?1034l,
	rmso=\E[27m,
	rmul=\E[24m,
	rs1=\Ec,
	rs2=\E[!p\E[?3;4l\E[4l\E>,
	sc=\E7,
	setab=\E[4%p1%dm,
	setaf=\E[3%p1%dm,
	setb=\E[4%?%p1%{1}%=%t4%e%p1%{3}%=%t6%e%p1%{4}%=%t1%e%p1%{6}%=%t3%e%p1%d%;m,
	setf=\E[3%?%p1%{1}%=%t4%e%p1%{3}%=%t6%e%p1%{4}%=%t1%e%p1%{6}%=%t3%e%p1%d%;m,
	sgr=%?%p9%t\E(0%e\E(B%;\E[0%?%p6%t;1%;%?%p5%t;2%;%?%p2%t;4%;%?%p1%p3%|%t;7%;%?%p4%t;5%;%?%p7%t;8%;m,
	sgr0=\E(B\E[m,
	sitm=\E[3m,
	smacs=\E(0,
	smam=\E[?7h,
	smcup=\E[?1049h\E[22;0;0t,
	smglr=\E[?69h\E[%i%p1%d;%p2%ds,
	smir=\E[4h,
	smkx=\E[?1h\E=,
	smm=\E[?1034h,
	smso=\E[7m,
	smul=\E[4m,
	tbc=\E[3g,
	u6=\E[%i%d;%dR,
	u7=\E[6n,
	u8=\E[?%[;0123456789]c,
	u9=\E[c,
	vpa=\E[%i%p1%dd,

@mdosch could you paste your output from Debian?

@bitstreamout
Copy link

Sidemark: ncurses here is configured with --enable-sigwinch, no idea if profanity uses a signal handler on SIGWINCH, but if so then it should also execute the old signal handler as well, if not set to SIG_IGN.

@jubalh
Copy link
Member Author

jubalh commented Dec 13, 2019

We have https://github.com/profanity-im/profanity/blob/master/src/ui/core.c#L145

    if (perform_resize) {
        signal(SIGWINCH, SIG_IGN);
        ui_resize();
        perform_resize = FALSE;
        signal(SIGWINCH, ui_sigwinch_handler);
    }

Is that okay?

@bitstreamout
Copy link

bitstreamout commented Dec 13, 2019

IMHO here the signal handler of libncurses is skipped as it is ignored.
sighandler_t oldwinch = signal(SIGWINCH, SIG_IGN);
and this might be executed if != SIG_IGN and != SIG_DFL within the `ui_sigwinch_handler'

More is found in `doc/ncurses-intro.doc' in section "Using NCURSES under XTERM"

@mdosch
Copy link
Contributor

mdosch commented Dec 13, 2019

Debian Bullseye (Testing)

#	Reconstructed via infocmp from file: /lib/terminfo/x/xterm
xterm|xterm-debian|X11 terminal emulator,
	am,
	bce,
	km,
	mc5i,
	mir,
	msgr,
	npc,
	xenl,
	colors#8,
	cols#80,
	it#8,
	lines#24,
	pairs#64,
	acsc=``aaffggiijjkkllmmnnooppqqrrssttuuvvwwxxyyzz{{||}}~~,
	bel=^G,
	blink=\E[5m,
	bold=\E[1m,
	cbt=\E[Z,
	civis=\E[?25l,
	clear=\E[H\E[2J,
	cnorm=\E[?12l\E[?25h,
	cr=\r,
	csr=\E[%i%p1%d;%p2%dr,
	cub=\E[%p1%dD,
	cub1=^H,
	cud=\E[%p1%dB,
	cud1=\n,
	cuf=\E[%p1%dC,
	cuf1=\E[C,
	cup=\E[%i%p1%d;%p2%dH,
	cuu=\E[%p1%dA,
	cuu1=\E[A,
	cvvis=\E[?12;25h,
	dch=\E[%p1%dP,
	dch1=\E[P,
	dim=\E[2m,
	dl=\E[%p1%dM,
	dl1=\E[M,
	ech=\E[%p1%dX,
	ed=\E[J,
	el=\E[K,
	el1=\E[1K,
	flash=\E[?5h$<100/>\E[?5l,
	home=\E[H,
	hpa=\E[%i%p1%dG,
	ht=^I,
	hts=\EH,
	ich=\E[%p1%d@,
	il=\E[%p1%dL,
	il1=\E[L,
	ind=\n,
	indn=\E[%p1%dS,
	invis=\E[8m,
	is2=\E[!p\E[?3;4l\E[4l\E>,
	kDC=\E[3;2~,
	kEND=\E[1;2F,
	kHOM=\E[1;2H,
	kIC=\E[2;2~,
	kLFT=\E[1;2D,
	kNXT=\E[6;2~,
	kPRV=\E[5;2~,
	kRIT=\E[1;2C,
	kb2=\EOE,
	kbs=^?,
	kcbt=\E[Z,
	kcub1=\EOD,
	kcud1=\EOB,
	kcuf1=\EOC,
	kcuu1=\EOA,
	kdch1=\E[3~,
	kend=\EOF,
	kent=\EOM,
	kf1=\EOP,
	kf10=\E[21~,
	kf11=\E[23~,
	kf12=\E[24~,
	kf13=\E[1;2P,
	kf14=\E[1;2Q,
	kf15=\E[1;2R,
	kf16=\E[1;2S,
	kf17=\E[15;2~,
	kf18=\E[17;2~,
	kf19=\E[18;2~,
	kf2=\EOQ,
	kf20=\E[19;2~,
	kf21=\E[20;2~,
	kf22=\E[21;2~,
	kf23=\E[23;2~,
	kf24=\E[24;2~,
	kf25=\E[1;5P,
	kf26=\E[1;5Q,
	kf27=\E[1;5R,
	kf28=\E[1;5S,
	kf29=\E[15;5~,
	kf3=\EOR,
	kf30=\E[17;5~,
	kf31=\E[18;5~,
	kf32=\E[19;5~,
	kf33=\E[20;5~,
	kf34=\E[21;5~,
	kf35=\E[23;5~,
	kf36=\E[24;5~,
	kf37=\E[1;6P,
	kf38=\E[1;6Q,
	kf39=\E[1;6R,
	kf4=\EOS,
	kf40=\E[1;6S,
	kf41=\E[15;6~,
	kf42=\E[17;6~,
	kf43=\E[18;6~,
	kf44=\E[19;6~,
	kf45=\E[20;6~,
	kf46=\E[21;6~,
	kf47=\E[23;6~,
	kf48=\E[24;6~,
	kf49=\E[1;3P,
	kf5=\E[15~,
	kf50=\E[1;3Q,
	kf51=\E[1;3R,
	kf52=\E[1;3S,
	kf53=\E[15;3~,
	kf54=\E[17;3~,
	kf55=\E[18;3~,
	kf56=\E[19;3~,
	kf57=\E[20;3~,
	kf58=\E[21;3~,
	kf59=\E[23;3~,
	kf6=\E[17~,
	kf60=\E[24;3~,
	kf61=\E[1;4P,
	kf62=\E[1;4Q,
	kf63=\E[1;4R,
	kf7=\E[18~,
	kf8=\E[19~,
	kf9=\E[20~,
	khome=\EOH,
	kich1=\E[2~,
	kind=\E[1;2B,
	kmous=\E[M,
	knp=\E[6~,
	kpp=\E[5~,
	kri=\E[1;2A,
	mc0=\E[i,
	mc4=\E[4i,
	mc5=\E[5i,
	meml=\El,
	memu=\Em,
	op=\E[39;49m,
	rc=\E8,
	rev=\E[7m,
	ri=\EM,
	rin=\E[%p1%dT,
	ritm=\E[23m,
	rmacs=\E(B,
	rmam=\E[?7l,
	rmcup=\E[?1049l\E[23;0;0t,
	rmir=\E[4l,
	rmkx=\E[?1l\E>,
	rmm=\E[?1034l,
	rmso=\E[27m,
	rmul=\E[24m,
	rs1=\Ec,
	rs2=\E[!p\E[?3;4l\E[4l\E>,
	sc=\E7,
	setab=\E[4%p1%dm,
	setaf=\E[3%p1%dm,
	setb=\E[4%?%p1%{1}%=%t4%e%p1%{3}%=%t6%e%p1%{4}%=%t1%e%p1%{6}%=%t3%e%p1%d%;m,
	setf=\E[3%?%p1%{1}%=%t4%e%p1%{3}%=%t6%e%p1%{4}%=%t1%e%p1%{6}%=%t3%e%p1%d%;m,
	sgr=%?%p9%t\E(0%e\E(B%;\E[0%?%p6%t;1%;%?%p5%t;2%;%?%p2%t;4%;%?%p1%p3%|%t;7%;%?%p4%t;5%;%?%p7%t;8%;m,
	sgr0=\E(B\E[m,
	sitm=\E[3m,
	smacs=\E(0,
	smam=\E[?7h,
	smcup=\E[?1049h\E[22;0;0t,
	smir=\E[4h,
	smkx=\E[?1h\E=,
	smm=\E[?1034h,
	smso=\E[7m,
	smul=\E[4m,
	tbc=\E[3g,
	u6=\E[%i%d;%dR,
	u7=\E[6n,
	u8=\E[?%[;0123456789]c,
	u9=\E[c,
	vpa=\E[%i%p1%dd,
echo $TERM
xterm-256color

@jubalh
Copy link
Member Author

jubalh commented Dec 13, 2019

diff -uwp debian opensuse
--- d1	2019-12-13 13:28:59.023301631 +0100
+++ d2	2019-12-13 13:29:13.223226012 +0100
@@ -1,5 +1,5 @@
-#	Reconstructed via infocmp from file: /lib/terminfo/x/xterm
-xterm|xterm-debian|X11 terminal emulator,
+#	Reconstructed via infocmp from file: /usr/share/terminfo/x/xterm
+xterm|xterm terminal emulator (X Window System),
 	am,
 	bce,
 	km,
@@ -62,8 +62,12 @@ xterm|xterm-debian|X11 terminal emulator
 	kNXT=\E[6;2~,
 	kPRV=\E[5;2~,
 	kRIT=\E[1;2C,
-	kb2=\EOE,
+	ka1=\EOw,
+	ka3=\EOy,
+	kb2=\EOu,
 	kbs=^?,
+	kc1=\EOq,
+	kc3=\EOs,
 	kcbt=\E[Z,
 	kcub1=\EOD,
 	kcud1=\EOB,
@@ -135,20 +139,24 @@ xterm|xterm-debian|X11 terminal emulator
 	kf7=\E[18~,
 	kf8=\E[19~,
 	kf9=\E[20~,
+	kfnd=\E[1~,
 	khome=\EOH,
 	kich1=\E[2~,
 	kind=\E[1;2B,
-	kmous=\E[M,
+	kmous=\E[<,
 	knp=\E[6~,
 	kpp=\E[5~,
 	kri=\E[1;2A,
+	kslt=\E[4~,
 	mc0=\E[i,
 	mc4=\E[4i,
 	mc5=\E[5i,
 	meml=\El,
 	memu=\Em,
+	mgc=\E[?69l,
 	op=\E[39;49m,
 	rc=\E8,
+	rep=%p1%c\E[%p2%{1}%-%db,
 	rev=\E[7m,
 	ri=\EM,
 	rin=\E[%p1%dT,
@@ -174,6 +182,7 @@ xterm|xterm-debian|X11 terminal emulator
 	smacs=\E(0,
 	smam=\E[?7h,
 	smcup=\E[?1049h\E[22;0;0t,
+	smglr=\E[?69h\E[%i%p1%d;%p2%ds,
 	smir=\E[4h,
 	smkx=\E[?1h\E=,
 	smm=\E[?1034h,

@bitstreamout
Copy link

The major difference seems to be rep=%p1%c\E[%p2%{1}%-%db as this is a major feature of XTerm to support, from changelog in misc/terminfo.src

 #  2017-07-29
 #       + update interix entry using tack and SFU on Windows 7 Ultimate -TD
 #       + use ^? for kdch1 in interix (reported by Jonathan de Boyne Pollard)
 #       + add "rep" to xterm-new, available since 1997/01/26 -TD
 #       + move SGR 24 and 27 from vte-2014 to vte-2012 (request by Alain
 #         Williams) -TD

@bitstreamout
Copy link

The "xterm-old" terminal description does not support the "rep" feature, that is if the misbehaviour is also visible then it is likely not the "rep" feature

@jubalh
Copy link
Member Author

jubalh commented Dec 17, 2019

IMHO here the signal handler of libncurses is skipped as it is ignored.
sighandler_t oldwinch = signal(SIGWINCH, SIG_IGN);
and this might be executed if != SIG_IGN and != SIG_DFL within the ui_sigwinch_handler' More is found in doc/ncurses-intro.doc' in section "Using NCURSES under XTERM"

Unfortunately I don't know why it was implemented like this. This was before I joined the project.

At https://github.com/profanity-im/profanity/blob/master/src/ui/core.c#L192 we have our own handler:

void
ui_resize(void)
{
    struct winsize w;
    ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
    erase();
    resizeterm(w.ws_row, w.ws_col);
    refresh();

    log_debug("Resizing UI");
    title_bar_resize();
    wins_resize_all();
    status_bar_resize();
    inp_win_resize();
    ProfWin *window = wins_get_current();
    win_update_virtual(window);
}

Also at this point the regular ncurses SIGWINCH handler is not called. but refresh() is called. Like mentioned in the documentation you linked. endwin is not called but I don't see why it should be.

Unfortunately I don't have a lot of knowledge of ncurses and "deep" terminal behaviour.

So right now I'm still uncertain on how to proceed with solving this issue :-/

@bitstreamout would you mind to advise me what I should do/check next?

@bitstreamout
Copy link

Check is TERM=xterm-old does work in XTerm as well in the other terminal emulators claiming to be XTerm compatible

@jubalh
Copy link
Member Author

jubalh commented Dec 17, 2019

Check is TERM=xterm-old does work in XTerm as well in the other terminal emulators claiming to be XTerm compatible

Also does not work properly. Tested in gnome-terminal. TERM=xterm ./profanity.

@bitstreamout
Copy link

Check is TERM=xterm-old does work in XTerm as well in the other terminal emulators claiming to be XTerm compatible

Also does not work properly. Tested in gnome-terminal. TERM=xterm ./profanity.

TERM=xterm-old ./profanity ... note the -old at the end ox xterm

@jubalh
Copy link
Member Author

jubalh commented Dec 17, 2019

Sorry.
I tried this again 3 times now. And it seems this works in gnome-terminal.

Edit:
Trying it a fourth time it suddenly does not:
showprof

Before the fourth time I resize the terminal with the mouse so that it was longer but not wider.

@bitstreamout
Copy link

Hmmm ... then this is not the missing rep= feature as it had been reported here https://bugzilla.gnome.org/show_bug.cgi?id=787701

@jubalh
Copy link
Member Author

jubalh commented Dec 20, 2019

Not sure how to continue here.
But since this only happens on openSUSE I'll remove it from the 0.8.0 milestone as I don't consider it a release blocker.

@jubalh jubalh removed this from the 0.8.0 milestone Dec 20, 2019
@jubalh jubalh added the bug label Dec 20, 2019
@egmontkob
Copy link

egmontkob commented Dec 22, 2019

(vte + gnome-terminal developer here)

Looks to me the combination of two issues:

On Wayland with GNOME Shell / Mutter and friends, CSD (client-side decorated) apps don't maximize/restore in a single step, they resize through an intermediate incorrect size. This is reported at GNOME Terminal 129 and is being worked on in Mutter 801. The third screenshot shows that Profanity paints its contents assuming a somewhat smaller screen size, apparently exactly the incorrect temporary size that VTE goes through. The problem doesn't occur on X11, or with SSD (server-side decorated) apps, for the latter check gnome-terminal's hidden "headerbar" setting in dconf.

That being said, Profanity should finally paint its contents according to the final window size. We're not aware of any race condition here in VTE: the window is resized and the kernel's tty size is always updated in an atomic step (or, well, two independent, complete atomic steps one after the other, as per the just-mentioned issue). However, we've already come across quite a few apps that fail to handle if multiple WINCHes arrive in quick succession, e.g. the second one arriving while the first one's handler is still running, or so. I'm pretty sure we're facing such a race condition in Profanity (or maybe ncurses) here; debugging what it's actually doing (e.g. strace'ing to record the WINCHes, catching with script what is sent to the terminal, and manually analyzing them) could help locate the actual problem.

@jubalh
Copy link
Member Author

jubalh commented Dec 23, 2019

@egmontkob ohhh!
So good to know about this.

However, we've already come across quite a few apps that fail to handle if multiple WINCHes arrive in quick succession, e.g. the second one arriving while the first one's handler is still running, or so.

Our handler is here: https://github.com/profanity-im/profanity/blob/master/src/ui/core.c#L192
And we have a check that prevents the handler from being run while a resize is taking place: https://github.com/profanity-im/profanity/blob/master/src/ui/core.c#L145

I guess this could be the condition you described.

I don't know why this check was actually added. It was before my time on this project.

@ghost
Copy link

ghost commented May 16, 2021

I also have this issue. I use profanity inside tmux. When the tmux window size changes, profanity sometimes resizes the window. Note that I tested this on very slow hardware (an old Raspberry Pi over a slow internet connection). My work-around, to force a complete window redraw, is as follows:

  • Create a split window in tmux.
  • Wait until profanity redrew everything. Given the slow hardware and the known inefficiency of the internal message buffer, this can take a few seconds.
  • Remove the split screen.
  • Again wait until profanity's window is completely redrawn.

In the short term, a shortcut like CTRL+L would be nice to force a complete redraw.

@jubalh jubalh added this to the 0.12.1 milestone Apr 1, 2022
@jubalh jubalh removed the help wanted label Apr 1, 2022
@jubalh
Copy link
Member Author

jubalh commented Apr 1, 2022

Fixed by #1671 it seems!

@jubalh jubalh closed this as completed Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants