Ticket #2452 (closed defect: fixed)
Doesn't save "Verbose operation" file operation option
Reported by: | nixtrian | Owned by: | zaytsev |
---|---|---|---|
Priority: | major | Milestone: | 4.8.33 |
Component: | mc-tty | Version: | master |
Keywords: | settings save Verbose operation removed | Cc: | |
Blocked By: | Blocking: | ||
Branch state: | merged | Votes for changeset: | committed-master |
Description
I launch mc, go into the "Options"/"Configuration" menu, put a marker on the "Verbose operation", then press save ("auto save setup" is marked). When I launch MC next time and go to "Options"/"Configuration" menu — there is no "Verbose operation" marker.
I tried to "rm -rf ~/.mc", there was no result.
$ LANG=C mc -V GNU Midnight Commander 4.7.5-pre1-16-g97ae800 Built with GLib 2.26.1 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, fish Data types: char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;
Change History
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 4 Changed 14 years ago by nixtrian
Replying to andrew_b:
What about other settings?
Other settings (I tested few) saving and loading normally
I am also tried to turn off autosave, then edited the ~/.mc/ini, but "Verbose operation" wasn't loaded.
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 14 years ago by andrew_b
Replying to nixtrian:
Replying to andrew_b:
What about other settings?
Other settings (I tested few) saving and loading normally
Looks like you terminal is slow. On slow terminal, the Verbose option is forced set to 0. This can occurs in 2 cases:
1) you launch mc with -s/--slow option;
2) S-Lang (or NCurses) determines that the output speed (the baud rate) of your terminal is less than 9600 bps.
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed 14 years ago by nixtrian
Replying to andrew_b:
Replying to nixtrian:
Replying to andrew_b:
What about other settings?
Other settings (I tested few) saving and loading normally
Looks like you terminal is slow. On slow terminal, the Verbose option is forced set to 0. This can occurs in 2 cases:
1) you launch mc with -s/--slow option;
2) S-Lang (or NCurses) determines that the output speed (the baud rate) of your terminal is less than 9600 bps.
I launch mc without -s/--slow option;
I use urxvt terminal with speed 4000000 baud (as "stty -a" says);
comment:6 in reply to: ↑ 5 Changed 14 years ago by nixtrian
I've downgraded urxvt now to 9.07 version. The MC works fine in it (saves and loads the subj option well). And stty says that terminal speed is 38400 bps. So maybe it is an urxvt bug.
comment:7 Changed 14 years ago by andrew_b
- Status changed from new to closed
- Keywords settings save Verbose operation removed
- Version master deleted
- Resolution set to invalid
- Milestone 4.8 deleted
OK. Closed.
comment:8 Changed 14 years ago by nixtrian
- Keywords settings save Verbose operation removed added
- Status changed from closed to reopened
- Version set to master
- Resolution invalid deleted
- Milestone set to 4.8
I think I should reopen this bug, cause I talked to urxvt developers and they confirmed the urxvt baudrate is 4000000.
I modified the midnight.c a bit (insert "printf("%i", tty_baudrate ());" before tty_baudrate check) to see what tty_baudrate returns and it returns "0" on the recent urxvt. So this is mc/slang bug (because "stty" returns the correct value (4000000) ).
comment:9 Changed 14 years ago by andrew_b
- Status changed from reopened to closed
- Resolution set to invalid
OK.
404 int 405 tty_baudrate (void) 406 { 407 return SLang_TT_Baud_Rate; 408 }
Since MC gets baud rate directly from S-Lang, I guess, this is bug of S-Lang. Please send a bugreport to S-Lang developer.
Would you build MC with NCurses and show the result here? Thanks!
comment:10 Changed 14 years ago by andrew_b
- Status changed from closed to reopened
- Resolution invalid deleted
comment:11 follow-up: ↓ 12 Changed 14 years ago by vsu
Reported the problem and the patch for S-Lang to the slang-users-l mailing list:
http://mailman.jedsoft.org/pipermail/slang-users-l/2011/000722.html
Applying this patch to S-Lang fixes the mc+urxvt problem for me.
comment:12 in reply to: ↑ 11 ; follow-up: ↓ 13 Changed 14 years ago by andrew_b
- Status changed from reopened to closed
- Component changed from mc-core to mc-tty
- Version master deleted
- Resolution set to invalid
- Milestone 4.8 deleted
Cool! Thanks!
Actually, this is not mc bug. Closed.
comment:13 in reply to: ↑ 12 Changed 14 years ago by fairplay
- Status changed from closed to reopened
- Resolution invalid deleted
Replying to andrew_b:
Cool! Thanks!
Actually, this is not mc bug. Closed.
It's definitely a mc bug, cause you should not turn off the verbose mode automatically on slow terminals, but you should leave it as an option.
comment:14 Changed 13 years ago by c0da
- Branch state set to no branch
+1 for disabling the automatic non-verbose mode
people with slow terminals should be able to add -s to their command line. there is no option to force verbose mode for those with urxvt (and maybe other terminals)
this problem also exists in the ncurses version of mc:
$ mc --version
GNU Midnight Commander 4.7.5.2
Built with GLib 2.28.8
Using the ncurses library
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, fish
Data types: char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;
$ set | grep TERM
COLORTERM=rxvt
TERM=xterm
$ stty
speed 4000000 baud; line = 0;
$ ncurses5-config --version
5.7.20081102
comment:15 Changed 13 years ago by c0da
gdb tells me that tty_baudrate () == -1
that is less than 9600 but a invalid value in my opinion, therefore this should not cause mc on modern systems to fallback to minimal functionality.
the issue can be solved with this patch. you have to enable verbose mode when it was disabled with a unpatched version.
diff --git a/src/filemanager/midnight.c b/src/filemanager/midnight.c index c92fe53..2019ac8 100644 --- a/src/filemanager/midnight.c +++ b/src/filemanager/midnight.c @@ -833,8 +833,7 @@ setup_mc (void) if (mc_global.tty.use_subshell) add_select_channel (mc_global.tty.subshell_pty, load_prompt, 0); #endif /* !HAVE_SUBSHELL_SUPPORT */ - - if ((tty_baudrate () < 9600) || mc_global.tty.slow_terminal) + if ((tty_baudrate () > 0 && tty_baudrate () < 9600) || mc_global.tty.slow_terminal) verbose = 0; }
comment:17 Changed 12 years ago by selecao
I found a solution:
stty 115200
just before starting mc (or somewhere in your scripts)
comment:18 Changed 5 weeks ago by zaytsev
- Owner set to zaytsev
- Status changed from reopened to accepted
- Milestone changed from Future Releases to 4.8.33
Screen libraries can return -1 if baudrate cannot be determined:
I think it makes sense to apply a check for >0, because then we can't reliably determine the speed and it's better to let user decide.
comment:19 Changed 5 weeks ago by zaytsev
- Branch state changed from no branch to on review
Branch: 2452_handle_baudrate_error
Changeset: 1ff8adc00593e20bd351d2b594cce1e7e8bd1bc1
comment:20 Changed 5 weeks ago by andrew_b
Double call of tty_baudrate() is not very good.
comment:21 follow-up: ↓ 22 Changed 5 weeks ago by zaytsev
Pushed a fixup.
I hope we can enable C99 one day, writing
const int baudrate = tty_baudrate ();
instead of
int baudrate; baudrate = tty_baudrate ();
enables massive constification, can help reduce the scopes and makes code more understandable / less verbose.
I wonder if you know any big important reason holding us back?
comment:22 in reply to: ↑ 21 Changed 5 weeks ago by andrew_b
Replying to zaytsev:
Pushed a fixup.
https://midnight-commander.org/wiki/Hacking
Don't mix variable declaration and code, declare variable only in the beginning of block. Split variable declaration and code using one empty line.
enables massive constification,
Constification of integer is pointless in almost all cases.
can help reduce the scopes
Scope reduce is irrelevant to constification.
and makes code more understandable / less verbose.
Likewise.
comment:23 Changed 5 weeks ago by zaytsev
I'm aware of that text, and that's why I'm asking you if you know why that rule was put in there. It just says don't do it, but it doesn't explain why.
My understanding is that this was done to support old broken pre-C99 compilers that do not support mixing code and declarations:
../../../src/filemanager/filemanager.c:857:15: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement] 857 | const int baudrate = tty_baudrate (); | ^
However, newer standards allow this, and there is a reason for it: to make the code safer. So I was wondering if you knew of any other reasons why it should not be done. If not, then maybe we can at least start requiring -std=c99.
Constification of integer is pointless in almost all cases.
This has nothing to do with ints. This applies to any type:
mc_type_t foo; // this forces to NOT make `foo` const, because foo = get_foo(); // it's initialized on the next line with a function call
There are a lot of places in mc where we are forced to initialise variables with some garbage like 0 because a function call would be a statement and that's not allowed.
I agree that const int rarely saves lives, especially if you pay attention to compiler warnings to avoid things like if (variable = TRUE) { ... }, but it does make it easier to reason about the code, knowing that the value of the variable cannot change after assignment. It's even more important for other types.
Scope reduce is irrelevant to constification.
It's not relevant to consification, but it's very relevant to mixing code and declarations. Right now we have tons of huge functions like:
type1_t var11, var12, ...; type2_t var21, var22, ...; { { // block 1 // var11, var12 only used here } { // block 2 // var21, var22 only used here } }
At the moment it's not possible to move variables to where they belong (reduce scopes), because that would mix code and declarations. On top of that, of course, you can make most of them const.
Refactoring big functions into small functions is good, but a first small step is to reduce scopes and join declaration & initialization.
Likewise.
Likewise what, you think that the following is bad?
- Reducing scopes
- Joining declaration and "real" initialization
- Making variables const
comment:24 Changed 3 weeks ago by zaytsev
As discussed, I have:
- Updated HACKING
- Updated NEWS
- Changed the build system
All platforms are green:
https://github.com/MidnightCommander/mc/actions/runs/11649942649
comment:25 Changed 3 weeks ago by andrew_b
'''This is right:''' {{{ void foo (const int iterations) { int result; do_one_thing (iterations); do_something (result); ... } }}} '''This is wrong:''' {{{ void foo (int iterations) { do_one_thing (iterations); do_something (iterations); ... } }}}
Unclear what's wrong. Do you mean
'''This is right:''' {{{ void foo (const int iterations) { int result; result = do_one_thing (iterations); do_something (result); ... } }}} '''This is wrong:''' {{{ void foo (int iterations) { iterations = do_one_thing (iterations); do_something (iterations); ... } }}}
comment:26 Changed 3 weeks ago by andrew_b
- Votes for changeset set to andrew_b
- Branch state changed from on review to approved
comment:27 Changed 3 weeks ago by zaytsev
- Status changed from accepted to testing
- Votes for changeset changed from andrew_b to committed-master
- Resolution set to fixed
- Branch state changed from approved to merged
Thanks, merged as c5b8b69371c2356d3c021f04d483c91c38040e5c .
There is redetermination: it saves settings well, but doesn't load it next start and rewrites in to default next shutdown.