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.

Sign up for GitHub

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

Jump to bottom

regression: resize/resizing window while command is executing is impartially felt by mc #4019

Closed
mc-butler opened this issue Sep 19, 2019 · 17 comments
Closed

regression: resize/resizing window while command is executing is impartially felt by mc #4019

mc-butler opened this issue Sep 19, 2019 · 17 comments
Assignees
@aborodin
Labels
area: tty Interaction with the terminal, screen libraries prio: low Minor problem or easily worked around
Milestone
4.8.24

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/4019
Reporter howaboutsynergy
Keywords window, resize

What steps will reproduce the problem?

  1. start 'mc' within an xfce4-terminal window
  2. run this command in 'mc': 
read -rp "press enter after resizing xfce4-terminal window"
  1. resize the window by making it larger in both X and Y directions
  2. press Enter to the prompt in 2.
  3. press Enter if prompted by 'mc' to "Press any key to continue..." to return to 'mc' panels

What is the expected output?

the panels are resized to the new window size

What do you see instead?

the panels are the same size as before the command in 2. got executed, but the lower bar (with the F1-F10 keys info) is resized correctly in the X direction.

(screenshots will be attached)

$ LC_MESSAGES=C mc -V
GNU Midnight Commander 4.8.23-75-g0ef1737aa
Built with GLib 2.62.0
Using the S-Lang library with terminfo database
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ftpfs, sftpfs, fish
Data types: char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;

$ LC_MESSAGES=C mc -F
Home directory: /home/user
Profile root directory: /home/user

[System data]
    Config directory: /etc/mc/
    Data directory:   /usr/share/mc/
    File extension handlers: /usr/lib/mc/ext.d/
    VFS plugins and scripts: /usr/lib/mc/
	extfs.d:        /usr/lib/mc/extfs.d/
	fish:           /usr/lib/mc/fish/

[User data]
    Config directory: /home/user/.config/mc/
    Data directory:   /home/user/.local/share/mc/
	skins:          /home/user/.local/share/mc/skins/
	extfs.d:        /home/user/.local/share/mc/extfs.d/
	fish:           /home/user/.local/share/mc/fish/
	mcedit macros:  /home/user/.local/share/mc/mc.macros
	mcedit external macros: /home/user/.local/share/mc/mcedit/macros.d/macro.*
    Cache directory:  /home/user/.cache/mc/

$ mc --configure-options
 '--prefix=/usr' '--sysconfdir=/etc' '--libexecdir=/usr/lib' '--enable-background' '--enable-network' '--enable-netcode' '--enable-charset' '--enable-nls' '--with-vfs' '--with-edit' '--with-screen=slang' '--without-x' '--without-samba' '--without-gpm-mouse' '--without-gnome' '--without-debug' '--without-included-gettext' '--disable-dependency-tracking' '--enable-tests'

Note

Original attachments:
@mc-butler
Copy link
Author

Changed by howaboutsynergy on Sep 19, 2019 at 15:21 UTC

step1 aka before

@mc-butler
Copy link
Author

Changed by howaboutsynergy on Sep 19, 2019 at 15:22 UTC

step 2&3 aka middle

@mc-butler
Copy link
Author

Changed by howaboutsynergy on Sep 19, 2019 at 15:23 UTC

after step 5

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 19, 2019 at 18:30 UTC (comment 1)

The regression was introduced in [37ffc02].

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 21, 2019 at 17:55 UTC (comment 2)

Before [37ffc02] we had mc_global.tty.winch_flag that was set in sigwinch_handler(), than it checked some times (note: checked but no reset) and finally reset in dialog_change_screen_size().

Currently we have tty_got_winch() function that checks whether SIGWINCH was raised. But this function has side effect: the virtual SIGWINCH flag is reset after read. 'Virtual flag' means the non-emptiness of FIFO. If tty_got_winch() is sequentially called several times, only first call can return TRUE (FIFO was not empty at this moment), other calls return FALSE (read from an empty FIFO). Finally, dialog_change_screen_size() may not be called in dialog event loop in frontend_dlg_run(). Such case we have if SIGWINCH is raised while subshell is active.

I've tried fix that in

branch: 4019_resize_fix
[4c36a5c8ea3386fbdf6b911986092fac11a0508a]

But I'm afraid it is overcomplicated compared to using a simple variable before [37ffc02].

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Sep 21, 2019 at 23:04 UTC (comment 3)

can't resist placing a big fat "told you so" here. :P

did something go wrong while committing the fix? what i see on trac cannot possibly be complete (two one-liners in two files, and it doesn't match the commit message).

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 22, 2019 at 4:33 UTC (comment 3.4)

Replying to ossi:

did something go wrong while committing the fix?

Indeed. Sorry. Fixed.

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Sep 22, 2019 at 10:56 UTC (comment 5)

ok, patch doesn't look bad (disclaimer: i did not examine the whole context the code lives in). some observations:

  • typo in commit message: "check whether it [i]s empty"
  • comment says "no timeout", but you actually mean "instant timeout" (or "don't block")
  • the original code had a second flush around line 530 in execute.c. that whole hack *might* be obsolete now, but that needs investigation.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 22, 2019 at 14:22 UTC (comment 5.6)

Replying to ossi:

  • typo in commit message: "check whether it [i]s empty"

Commit message will be fixed before merge.

  • comment says "no timeout", but you actually mean "instant timeout" (or "don't block")

Fixed

  • the original code had a second flush around line 530 in execute.c. that whole hack *might* be obsolete now, but that needs investigation.

Restored.

@mc-butler
Copy link
Author

Changed by howaboutsynergy on Sep 24, 2019 at 19:13 UTC (comment 7)

I tried this https://github.com/MidnightCommander/mc/compare/4019_resize_fix.patch on top of 4.8.23.82.gb633256c7-1 and it works! Thank you.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 29, 2019 at 8:16 UTC (comment 8)

  • Votes set to howaboutsynergy

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 29, 2019 at 8:17 UTC (comment 9)

  • Milestone changed from Future Releases to 4.8.24
  • Votes changed from howaboutsynergy to howaboutsynergy andrew_b
  • Branch state changed from no branch to approved

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 29, 2019 at 8:18 UTC (comment 10)

  • Resolution set to fixed
  • Status changed from new to closed
  • Votes changed from howaboutsynergy andrew_b to committed-master
  • Branch state changed from approved to merged

Merged to master: [2cf2a18].

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 29, 2019 at 8:19 UTC (comment 11)

  • Resolution fixed deleted
  • Status changed from closed to reopened

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 29, 2019 at 8:20 UTC (comment 12)

  • Owner set to andrew_b
  • Status changed from reopened to accepted

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 29, 2019 at 8:21 UTC (comment 13)

  • Status changed from accepted to testing
  • Resolution set to fixed

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Sep 29, 2019 at 8:21 UTC (comment 14)

  • Status changed from testing to closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aborodin aborodin

Labels
area: tty Interaction with the terminal, screen libraries prio: low Minor problem or easily worked around
Milestone
4.8.24
Development

No branches or pull requests
2 participants
@aborodin @mc-butler