Doesn't save "Verbose operation" file operation option

[nixtrian]

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;

[nixtrian]

There is redetermination: it saves settings well, but doesn't load it next start and rewrites in to default next shutdown.

[andrew_b]

What about other settings?

[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.

[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.

[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);

[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.

[andrew_b]

OK. Closed.

[nixtrian]

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) ).

[andrew_b]

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!

[vsu]

Reported the problem and the patch for S-Lang to the slang-users-l mailing list:

​http://lists.jedsoft.org/lists/slang-users/2011/0000001.html

Applying this patch to S-Lang fixes the mc+urxvt problem for me.

[andrew_b]

Cool! Thanks!

Actually, this is not mc bug. Closed.

[fairplay]

Replying to andrew_b:

Cool! Thanks!

Actually, this is not mc bug. Closed.

It's definitely a mc bug, cause you should not automatically turn off the verbose mode on slow terminals, but you should leave it as option.

[c0da]

+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

[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;
 }

[selecao]

I found a solution:

stty 115200

just before starting mc (or somewhere in your scripts)

[zaytsev]

Screen libraries can return -1 if baudrate cannot be determined:

​https://github.com/mirror/ncurses/blob/87c2c84cbd2332d6d94b12a1dcaf12ad1a51a938/ncurses/tinfo/lib_baudrate.c#L207

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.

[zaytsev]

Branch: 2452_handle_baudrate_error
Changeset: 1ff8adc00593e20bd351d2b594cce1e7e8bd1bc1

[andrew_b]

Double call of tty_baudrate() is not very good.

[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?

[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.

[zaytsev]

https://midnight-commander.org/wiki/Hacking

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

[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

[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);
    ...
 }
}}}

[zaytsev]

Thanks, merged as c5b8b69371c2356d3c021f04d483c91c38040e5c .